From: Jan Kara <jack@suse.cz>
inode_to_wb() is used also for filesystems that don't support cgroup
writeback. For these filesystems inode->i_wb is stable during the
lifetime of the inode (it points to bdi->wb) and there's no need to hold
locks protecting the inode->i_wb dereference. Improve the warning in
inode_to_wb() to not trigger for these filesystems.
Signed-off-by: Jan Kara <jack@suse.cz>
Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com>
---
include/linux/backing-dev.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index 8e7af9a03b41..e721148c95d0 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -249,6 +249,7 @@ static inline struct bdi_writeback *inode_to_wb(const struct inode *inode)
{
#ifdef CONFIG_LOCKDEP
WARN_ON_ONCE(debug_locks &&
+ (inode->i_sb->s_iflags & SB_I_CGROUPWB) &&
(!lockdep_is_held(&inode->i_lock) &&
!lockdep_is_held(&inode->i_mapping->i_pages.xa_lock) &&
!lockdep_is_held(&inode->i_wb->list_lock)));
--
2.48.1
On Sat, 12 Apr 2025 18:39:12 +0200 Andreas Gruenbacher <agruenba@redhat.com> wrote: > inode_to_wb() is used also for filesystems that don't support cgroup > writeback. For these filesystems inode->i_wb is stable during the > lifetime of the inode (it points to bdi->wb) and there's no need to hold > locks protecting the inode->i_wb dereference. Improve the warning in > inode_to_wb() to not trigger for these filesystems. > > Signed-off-by: Jan Kara <jack@suse.cz> > Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com> Yoo were on the patch delivery path so there should be a signed-off-by:Andreas somewhere. I made that change to the mm.git copy of this patch.
On Mon, Apr 14, 2025 at 11:51 PM Andrew Morton <akpm@linux-foundation.org> wrote: > On Sat, 12 Apr 2025 18:39:12 +0200 Andreas Gruenbacher <agruenba@redhat.com> wrote: > > > inode_to_wb() is used also for filesystems that don't support cgroup > > writeback. For these filesystems inode->i_wb is stable during the > > lifetime of the inode (it points to bdi->wb) and there's no need to hold > > locks protecting the inode->i_wb dereference. Improve the warning in > > inode_to_wb() to not trigger for these filesystems. > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com> > > Yoo were on the patch delivery path so there should be a > signed-off-by:Andreas somewhere. I made that change to the mm.git copy > of this patch. I guess that's fine as long as Jan is credited as the author. Thanks, Andreas
On Sat, 12 Apr 2025 18:39:12 +0200 Andreas Gruenbacher <agruenba@redhat.com> wrote:
> From: Jan Kara <jack@suse.cz>
>
> inode_to_wb() is used also for filesystems that don't support cgroup
> writeback. For these filesystems inode->i_wb is stable during the
> lifetime of the inode (it points to bdi->wb) and there's no need to hold
> locks protecting the inode->i_wb dereference. Improve the warning in
> inode_to_wb() to not trigger for these filesystems.
>
> ...
>
> --- a/include/linux/backing-dev.h
> +++ b/include/linux/backing-dev.h
> @@ -249,6 +249,7 @@ static inline struct bdi_writeback *inode_to_wb(const struct inode *inode)
> {
> #ifdef CONFIG_LOCKDEP
> WARN_ON_ONCE(debug_locks &&
> + (inode->i_sb->s_iflags & SB_I_CGROUPWB) &&
> (!lockdep_is_held(&inode->i_lock) &&
> !lockdep_is_held(&inode->i_mapping->i_pages.xa_lock) &&
> !lockdep_is_held(&inode->i_wb->list_lock)));
Is this a does-nothing now GFS2 has been altered?
Otherwise, a bogus WARN is something we'll want to eliminate from
-stable kernels also. Are we able to identify a Fixes: for this?
Thanks.
On Mon, Apr 14, 2025 at 11:47 PM Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Sat, 12 Apr 2025 18:39:12 +0200 Andreas Gruenbacher <agruenba@redhat.com> wrote:
> > From: Jan Kara <jack@suse.cz>
> >
> > inode_to_wb() is used also for filesystems that don't support cgroup
> > writeback. For these filesystems inode->i_wb is stable during the
> > lifetime of the inode (it points to bdi->wb) and there's no need to hold
> > locks protecting the inode->i_wb dereference. Improve the warning in
> > inode_to_wb() to not trigger for these filesystems.
> >
> > ...
> >
> > --- a/include/linux/backing-dev.h
> > +++ b/include/linux/backing-dev.h
> > @@ -249,6 +249,7 @@ static inline struct bdi_writeback *inode_to_wb(const struct inode *inode)
> > {
> > #ifdef CONFIG_LOCKDEP
> > WARN_ON_ONCE(debug_locks &&
> > + (inode->i_sb->s_iflags & SB_I_CGROUPWB) &&
> > (!lockdep_is_held(&inode->i_lock) &&
> > !lockdep_is_held(&inode->i_mapping->i_pages.xa_lock) &&
> > !lockdep_is_held(&inode->i_wb->list_lock)));
>
> Is this a does-nothing now GFS2 has been altered?
>
> Otherwise, a bogus WARN is something we'll want to eliminate from
> -stable kernels also. Are we able to identify a Fixes: for this?
The excess warnings started with commit:
Fixes: aaa2cacf8184 ("writeback: add lockdep annotation to inode_to_wb()")
Getting rid of them requires this change, together with "gfs2: replace
sd_aspace with sd_inode" from gfs2 for-next:
https://web.git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/commit/?h=
a5fb828aba730d08aa6dec6bce3839f25e1f7a9d
Thanks,
Andreas
Hi Andrew!
Can you please take this patch through MM tree? Thanks! Andreas is already
taking the first patch in the series through GFS2 tree.
Honza
On Sat 12-04-25 18:39:12, Andreas Gruenbacher wrote:
> From: Jan Kara <jack@suse.cz>
>
> inode_to_wb() is used also for filesystems that don't support cgroup
> writeback. For these filesystems inode->i_wb is stable during the
> lifetime of the inode (it points to bdi->wb) and there's no need to hold
> locks protecting the inode->i_wb dereference. Improve the warning in
> inode_to_wb() to not trigger for these filesystems.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> include/linux/backing-dev.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
> index 8e7af9a03b41..e721148c95d0 100644
> --- a/include/linux/backing-dev.h
> +++ b/include/linux/backing-dev.h
> @@ -249,6 +249,7 @@ static inline struct bdi_writeback *inode_to_wb(const struct inode *inode)
> {
> #ifdef CONFIG_LOCKDEP
> WARN_ON_ONCE(debug_locks &&
> + (inode->i_sb->s_iflags & SB_I_CGROUPWB) &&
> (!lockdep_is_held(&inode->i_lock) &&
> !lockdep_is_held(&inode->i_mapping->i_pages.xa_lock) &&
> !lockdep_is_held(&inode->i_wb->list_lock)));
> --
> 2.48.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
© 2016 - 2025 Red Hat, Inc.