mbox series

[v5,0/3] Remove the XFS mrlock

Message ID 20240111212424.3572189-1-willy@infradead.org (mailing list archive)
Headers show
Series Remove the XFS mrlock | expand

Message

Matthew Wilcox Jan. 11, 2024, 9:24 p.m. UTC
XFS has an mrlock wrapper around the rwsem which adds only the
functionality of knowing whether the rwsem is currently held in read
or write mode.  Both regular rwsems and rt-rwsems know this, they just
don't expose it as an API.  By adding that, we can remove the XFS mrlock
as well as improving the debug assertions for the mmap_lock when lockdep
is disabled.

I have an ack on the first patch from Peter, so I would like to see this
merged through the XFS tree since most of what it touches is XFS.

v5:
 - Rebase on 5bad490858c3 (current Linus head) to pick up XFS changes

v4:
 - Switch the BUG_ONs to WARN_ONs (Wayman, Peter)

v3:
 - Rename __rwsem_assert_held() and __rwsem_assert_held_write() to
   rwsem_assert_held*_nolockdep()
 - Use IS_ENABLED(CONFIG_LOCKDEP) to only dump the information once
 - Use ASSERT instead of BUG_ON in xfs
 - Fix typo in subject line of patch 4
 - Drop patch 5 (inode_assert_locked)
 - Rebase on top of xfs-6.7-merge-2 which had a merge conflict

v2: Add rwsem_assert_held() and rwsem_assert_held_write() instead of
augmenting the existing rwsem_is_locked() with rwsem_is_write_locked().
There's also an __rwsem_assert_held() and __rwsem_assert_held_write()
for the benefit of XFS when it's in a context where lockdep doesn't
know what's going on.  It's still an improvement, so I hope those who
are looking for perfection can accept a mere improvement.

Matthew Wilcox (Oracle) (3):
  locking: Add rwsem_assert_held() and rwsem_assert_held_write()
  xfs: Replace xfs_isilocked with xfs_assert_ilocked
  xfs: Remove mrlock wrapper

 fs/xfs/libxfs/xfs_attr.c        |  2 +-
 fs/xfs/libxfs/xfs_attr_remote.c |  2 +-
 fs/xfs/libxfs/xfs_bmap.c        | 21 ++++----
 fs/xfs/libxfs/xfs_defer.c       |  2 +-
 fs/xfs/libxfs/xfs_inode_fork.c  |  2 +-
 fs/xfs/libxfs/xfs_rtbitmap.c    |  2 +-
 fs/xfs/libxfs/xfs_trans_inode.c |  6 +--
 fs/xfs/mrlock.h                 | 78 ------------------------------
 fs/xfs/scrub/readdir.c          |  4 +-
 fs/xfs/xfs_attr_list.c          |  2 +-
 fs/xfs/xfs_bmap_util.c          | 10 ++--
 fs/xfs/xfs_dir2_readdir.c       |  2 +-
 fs/xfs/xfs_dquot.c              |  4 +-
 fs/xfs/xfs_file.c               |  4 +-
 fs/xfs/xfs_inode.c              | 86 ++++++++++++---------------------
 fs/xfs/xfs_inode.h              |  4 +-
 fs/xfs/xfs_inode_item.c         |  4 +-
 fs/xfs/xfs_iops.c               |  7 ++-
 fs/xfs/xfs_linux.h              |  2 +-
 fs/xfs/xfs_qm.c                 | 10 ++--
 fs/xfs/xfs_reflink.c            |  2 +-
 fs/xfs/xfs_rtalloc.c            |  2 +-
 fs/xfs/xfs_super.c              |  4 +-
 fs/xfs/xfs_symlink.c            |  2 +-
 fs/xfs/xfs_trans.c              |  2 +-
 fs/xfs/xfs_trans_dquot.c        |  2 +-
 include/linux/rwbase_rt.h       |  9 +++-
 include/linux/rwsem.h           | 46 ++++++++++++++++--
 28 files changed, 129 insertions(+), 194 deletions(-)
 delete mode 100644 fs/xfs/mrlock.h

Comments

Dave Chinner Jan. 16, 2024, 3:07 a.m. UTC | #1
On Thu, Jan 11, 2024 at 09:24:21PM +0000, Matthew Wilcox (Oracle) wrote:
> XFS has an mrlock wrapper around the rwsem which adds only the
> functionality of knowing whether the rwsem is currently held in read
> or write mode.  Both regular rwsems and rt-rwsems know this, they just
> don't expose it as an API.  By adding that, we can remove the XFS mrlock
> as well as improving the debug assertions for the mmap_lock when lockdep
> is disabled.
> 
> I have an ack on the first patch from Peter, so I would like to see this
> merged through the XFS tree since most of what it touches is XFS.

With the minor nits that have already been noticed fix up, the whole
series looks good to me.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
Matthew Wilcox Feb. 14, 2024, 10:05 p.m. UTC | #2
On Thu, Jan 11, 2024 at 09:24:21PM +0000, Matthew Wilcox (Oracle) wrote:
> XFS has an mrlock wrapper around the rwsem which adds only the
> functionality of knowing whether the rwsem is currently held in read
> or write mode.  Both regular rwsems and rt-rwsems know this, they just
> don't expose it as an API.  By adding that, we can remove the XFS mrlock
> as well as improving the debug assertions for the mmap_lock when lockdep
> is disabled.
> 
> I have an ack on the first patch from Peter, so I would like to see this
> merged through the XFS tree since most of what it touches is XFS.

What needs to happen to get these picked up to not miss the next merge
window?
Darrick J. Wong Feb. 14, 2024, 10:17 p.m. UTC | #3
On Wed, Feb 14, 2024 at 10:05:10PM +0000, Matthew Wilcox wrote:
> On Thu, Jan 11, 2024 at 09:24:21PM +0000, Matthew Wilcox (Oracle) wrote:
> > XFS has an mrlock wrapper around the rwsem which adds only the
> > functionality of knowing whether the rwsem is currently held in read
> > or write mode.  Both regular rwsems and rt-rwsems know this, they just
> > don't expose it as an API.  By adding that, we can remove the XFS mrlock
> > as well as improving the debug assertions for the mmap_lock when lockdep
> > is disabled.
> > 
> > I have an ack on the first patch from Peter, so I would like to see this
> > merged through the XFS tree since most of what it touches is XFS.
> 
> What needs to happen to get these picked up to not miss the next merge
> window?

Rebase against xfs-linux for-next, then send Chandan a pull request.

That would have gotten done this morning, only now everything's on hold
again while we wait for the mm maintainers to get around to reviewing
the patches in the xfile diet v3 series:
https://lore.kernel.org/linux-xfs/87frxva65g.fsf@debian-BULLSEYE-live-builder-AMD64/T/#m99f2f7e1fb221c4d7f5a00f249119d570ab70fce

Yes I know you already reviewed all of them (again, thank you!) but
apparently your RVB tag is not good enough for the very high standards
of the kernel community.  In the mean time, the only thing we can do is
wait patiently and hope that one of the maintainers can free up enough
time to take a look.

</sarcasm>

--D
Chandan Babu R Feb. 15, 2024, 2:23 p.m. UTC | #4
On Wed, Feb 14, 2024 at 10:05:10 PM +0000, Matthew Wilcox wrote:
> On Thu, Jan 11, 2024 at 09:24:21PM +0000, Matthew Wilcox (Oracle) wrote:
>> XFS has an mrlock wrapper around the rwsem which adds only the
>> functionality of knowing whether the rwsem is currently held in read
>> or write mode.  Both regular rwsems and rt-rwsems know this, they just
>> don't expose it as an API.  By adding that, we can remove the XFS mrlock
>> as well as improving the debug assertions for the mmap_lock when lockdep
>> is disabled.
>> 
>> I have an ack on the first patch from Peter, so I would like to see this
>> merged through the XFS tree since most of what it touches is XFS.
>
> What needs to happen to get these picked up to not miss the next merge
> window?

Sorry, I missed this patchset. I tried applying this patchset on top of XFS'
current for-next branch. But it failed due to merge conflicts. I would suggest
that you should wait until the "shmem patches" impasse gets resolved and
for-next branch becomes stable.

I will respond to this mail as to when you can rebase your patchset on
for-next and either send a pull request or post the rebased version of the
patchset to the mailing list.
Chandan Babu R Feb. 19, 2024, 3:06 p.m. UTC | #5
On Wed, Feb 14, 2024 at 10:05:10 PM +0000, Matthew Wilcox wrote:
> On Thu, Jan 11, 2024 at 09:24:21PM +0000, Matthew Wilcox (Oracle) wrote:
>> XFS has an mrlock wrapper around the rwsem which adds only the
>> functionality of knowing whether the rwsem is currently held in read
>> or write mode.  Both regular rwsems and rt-rwsems know this, they just
>> don't expose it as an API.  By adding that, we can remove the XFS mrlock
>> as well as improving the debug assertions for the mmap_lock when lockdep
>> is disabled.
>> 
>> I have an ack on the first patch from Peter, so I would like to see this
>> merged through the XFS tree since most of what it touches is XFS.
>
> What needs to happen to get these picked up to not miss the next merge
> window?

Matthew, I have updated XFS' for-next branch. The HEAD commit is now
49c379d3a72ab86aafeafebe6b43577acb1ef359 ("xfs: use kvfree for buf in
xfs_ioc_getbmap"). Pleae rebase your patchset on top of for-next and either
post the rebased patchset or you can send me a pull request.