[RFC 0/2] Fix false warning in inode_to_wb

[Andreas Gruenbacher <agruenba@redhat.com>]

Hello,

when CONFIG_LOCKDEP is enabled, gfs2 triggers the following warning in
inode_to_wb() (include/linux/backing-dev.h) for a number of
filesystem-internal inodes:

  static inline struct bdi_writeback *inode_to_wb(const struct inode *inode)
  {
  #ifdef CONFIG_LOCKDEP
          WARN_ON_ONCE(debug_locks &&
                       (!lockdep_is_held(&inode->i_lock) &&
                        !lockdep_is_held(&inode->i_mapping->i_pages.xa_lock) &&
                        !lockdep_is_held(&inode->i_wb->list_lock)));
  #endif
          return inode->i_wb;
  }

This unfortunately makes lockdep unusable for gfs2.

In the most recent bug report about that problem [1], Jan Kara dug into
this and he concluded that when cgroup writeback is disabled, inode->i_wb
should be stable without any additional locking and the warnings are not
justified.  So can we please add an inode_cgwb_enabled() check to
inode_to_wb() as in Jan's patch in this series?


With that, a minor problem remains at the filesystem level:

Cgroup writeback is only enabled on filesystems that enable the
SB_I_CGROUPWB super block flag.  Unfortunately, gfs2 creates a separate
address space for filesystem metadata (sd_aspace) and sets its
mapping->host to sb->s_bdev->bd_mapping->host.  That's a "bdev" inode
with a super block that has SB_I_CGROUPWB set.  I'm not aware of any
other filesystems doing that.

To fix that, the first patch in this series creates an anonymous inode
instead of an isolated address space.  In that inode, ->i_mapping->host
points back at the inode and ->i_sb points at a gfs2 super block which
and doesn't have SB_I_CGROUPWB set.


And then there is another peculiarity of gfs2: while an 'ordinary' inode
has one address space for managing the inode's page cache, a gfs2 inode
also has a second address space for managing the inode's metadata cache.
These address spaces also point at sb->s_bdev->bd_mapping->host, causing
the same problem as above.  To fix that, the first patch changes ->host
of those address spaces to point at the new anonymous inode as well.

Using address spaces in this particular way seems to be pretty unusual
and there's a real risk that it will break some day, but I haven't found
a reasonable alternative so far.


Two previous discussions about this bug can be found here:

  [1] https://lore.kernel.org/gfs2/ebfe4024-f908-458d-9979-6ff2931c460d@I-love.SAKURA.ne.jp/
  [2] https://lore.kernel.org/all/20210713180958.66995-11-rpeterso@redhat.com/


Thanks,
Andreas


Andreas Gruenbacher (1):
  gfs2: replace sd_aspace with sd_inode

Jan Kara (1):
  writeback: Fix false warning in inode_to_wb()

 fs/gfs2/glock.c             |  3 +--
 fs/gfs2/glops.c             |  4 ++--
 fs/gfs2/incore.h            |  9 ++++++++-
 fs/gfs2/meta_io.c           |  2 +-
 fs/gfs2/meta_io.h           |  4 +---
 fs/gfs2/ops_fstype.c        | 24 +++++++++++++-----------
 fs/gfs2/super.c             |  2 +-
 include/linux/backing-dev.h |  3 ++-
 8 files changed, 29 insertions(+), 22 deletions(-)

-- 
2.48.1