On Wed, Jun 24, 2020 at 11:36:52AM -0400, Brian Foster wrote: > On Tue, Jun 23, 2020 at 07:50:12PM +1000, Dave Chinner wrote: > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > With the recent rework of the inode cluster flushing, we no longer > > ever wait on the the inode flush "lock". It was never a lock in the > > first place, just a completion to allow callers to wait for inode IO > > to complete. We now never wait for flush completion as all inode > > flushing is non-blocking. Hence we can get rid of all the iflock > > infrastructure and instead just set and check a state flag. > > > > Rename the XFS_IFLOCK flag to XFS_IFLUSHING, convert all the > > xfs_iflock_nowait() test-and-set operations on that flag, and > > replace all the xfs_ifunlock() calls to clear operations. > > > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > > --- > > I'd call it IFLUSHED vs. IFLUSHING, but I'm not going to harp on that. > Just a few nits, otherwise looks like a nice cleanup. I thought about "flushed" but that implies something that has alerady happened in the past (e.g. XFS_ITRUNCATED) rather than something that is currently happening. i.e. we are wanting to know if a flush is in progress right now, not whether a flush has been done in the past... > > fs/xfs/xfs_icache.c | 19 ++++++------ > > fs/xfs/xfs_inode.c | 67 +++++++++++++++-------------------------- > > fs/xfs/xfs_inode.h | 33 +------------------- > > fs/xfs/xfs_inode_item.c | 6 ++-- > > fs/xfs/xfs_inode_item.h | 4 +-- > > 5 files changed, 39 insertions(+), 90 deletions(-) > > > > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c > > index a973f180c6cd..0d73559f2d58 100644 > > --- a/fs/xfs/xfs_icache.c > > +++ b/fs/xfs/xfs_icache.c > ... > > @@ -1045,23 +1044,23 @@ xfs_reclaim_inode( > > > > if (!xfs_ilock_nowait(ip, XFS_ILOCK_EXCL)) > > goto out; > > - if (!xfs_iflock_nowait(ip)) > > + if (xfs_iflags_test_and_set(ip, XFS_IFLUSHING)) > > goto out_iunlock; > > > > if (XFS_FORCED_SHUTDOWN(ip->i_mount)) { > > xfs_iunpin_wait(ip); > > /* xfs_iflush_abort() drops the flush lock */ > > The flush what? ;P > > > xfs_iflush_abort(ip); > > + ASSERT(!xfs_iflags_test(ip, XFS_IFLUSHING)); > > Seems a bit superfluous right after the abort. Yup, I'll that clean up and the comments the search and replace I did missed. Thanks! -Dave. -- Dave Chinner david@xxxxxxxxxxxxx