Message ID | 162259518016.662681.13322964506776234493.stgit@locust (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | xfs: clean up incore inode walk functions | expand |
On Tue, Jun 01, 2021 at 05:53:00PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > Disentangle the dqrele_all inode grab code from the "generic" inode walk > grabbing code, and and use the opportunity to document why the dqrele > grab function does what it does. Since xfs_inode_walk_ag_grab is now > only used for blockgc, rename it to reflect that. > > Ultimately, there will be four reasons to perform a walk of incore > inodes: quotaoff dquote releasing (dqrele), garbage collection of > speculative preallocations (blockgc), reclamation of incore inodes > (reclaim), and deferred inactivation (inodegc). Each of these four have > their own slightly different criteria for deciding if they want to > handle an inode, so it makes more sense to have four cohesive igrab > functions than one confusing parameteric grab function like we do now. > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/xfs_icache.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 61 insertions(+), 5 deletions(-) Looks ok - one minor nit: > @@ -1642,6 +1682,22 @@ xfs_blockgc_free_quota( > > /* XFS Incore Inode Walking Code */ > > +static inline bool > +xfs_grabbed_for_walk( > + enum xfs_icwalk_goal goal, > + struct xfs_inode *ip, > + int iter_flags) > +{ > + switch (goal) { > + case XFS_ICWALK_BLOCKGC: > + return xfs_blockgc_igrab(ip, iter_flags); > + case XFS_ICWALK_DQRELE: > + return xfs_dqrele_igrab(ip); > + default: > + return false; > + } > +} xfs_icwalk_grab() seems to make more sense here. /me is wondering if all this should eventually end up under a xfs_icwalk namespace? Cheers, Dave.
On Wed, Jun 02, 2021 at 11:51:47AM +1000, Dave Chinner wrote: > On Tue, Jun 01, 2021 at 05:53:00PM -0700, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > Disentangle the dqrele_all inode grab code from the "generic" inode walk > > grabbing code, and and use the opportunity to document why the dqrele > > grab function does what it does. Since xfs_inode_walk_ag_grab is now > > only used for blockgc, rename it to reflect that. > > > > Ultimately, there will be four reasons to perform a walk of incore > > inodes: quotaoff dquote releasing (dqrele), garbage collection of > > speculative preallocations (blockgc), reclamation of incore inodes > > (reclaim), and deferred inactivation (inodegc). Each of these four have > > their own slightly different criteria for deciding if they want to > > handle an inode, so it makes more sense to have four cohesive igrab > > functions than one confusing parameteric grab function like we do now. > > > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > fs/xfs/xfs_icache.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 61 insertions(+), 5 deletions(-) > > Looks ok - one minor nit: > > > @@ -1642,6 +1682,22 @@ xfs_blockgc_free_quota( > > > > /* XFS Incore Inode Walking Code */ > > > > +static inline bool > > +xfs_grabbed_for_walk( > > + enum xfs_icwalk_goal goal, > > + struct xfs_inode *ip, > > + int iter_flags) > > +{ > > + switch (goal) { > > + case XFS_ICWALK_BLOCKGC: > > + return xfs_blockgc_igrab(ip, iter_flags); > > + case XFS_ICWALK_DQRELE: > > + return xfs_dqrele_igrab(ip); > > + default: > > + return false; > > + } > > +} > > xfs_icwalk_grab() seems to make more sense here. Ok, changed. > /me is wondering if all this should eventually end up under a > xfs_icwalk namespace? Yeah, I'll throw a renamer patch on the end of this series. Or possibly just do it after I move the xfs_inode_walk functions to the bottom. --D > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c index 018b3f8bdd21..bc88d33f7f24 100644 --- a/fs/xfs/xfs_icache.c +++ b/fs/xfs/xfs_icache.c @@ -757,6 +757,44 @@ xfs_icache_inode_is_allocated( #define XFS_LOOKUP_BATCH 32 #ifdef CONFIG_XFS_QUOTA +/* Decide if we want to grab this inode to drop its dquots. */ +static bool +xfs_dqrele_igrab( + struct xfs_inode *ip) +{ + bool ret = false; + + ASSERT(rcu_read_lock_held()); + + /* Check for stale RCU freed inode */ + spin_lock(&ip->i_flags_lock); + if (!ip->i_ino) + goto out_unlock; + + /* + * Skip inodes that are anywhere in the reclaim machinery because we + * drop dquots before tagging an inode for reclamation. + */ + if (ip->i_flags & (XFS_IRECLAIM | XFS_IRECLAIMABLE)) + goto out_unlock; + + /* + * The inode looks alive; try to grab a VFS reference so that it won't + * get destroyed. If we got the reference, return true to say that + * we grabbed the inode. + * + * If we can't get the reference, then we know the inode had its VFS + * state torn down and hasn't yet entered the reclaim machinery. Since + * we also know that dquots are detached from an inode before it enters + * reclaim, we can skip the inode. + */ + ret = igrab(VFS_I(ip)) != NULL; + +out_unlock: + spin_unlock(&ip->i_flags_lock); + return ret; +} + /* Drop this inode's dquots. */ static int xfs_dqrele_inode( @@ -806,6 +844,8 @@ xfs_dqrele_all_inodes( return xfs_inode_walk(mp, XFS_INODE_WALK_INEW_WAIT, xfs_dqrele_inode, &eofb, XFS_ICWALK_DQRELE); } +#else +# define xfs_dqrele_igrab(ip) (false) #endif /* CONFIG_XFS_QUOTA */ /* @@ -1478,12 +1518,12 @@ xfs_blockgc_start( } /* - * Decide if the given @ip is eligible to be a part of the inode walk, and - * grab it if so. Returns true if it's ready to go or false if we should just - * ignore it. + * Decide if the given @ip is eligible for garbage collection of speculative + * preallocations, and grab it if so. Returns true if it's ready to go or + * false if we should just ignore it. */ static bool -xfs_inode_walk_ag_grab( +xfs_blockgc_igrab( struct xfs_inode *ip, int flags) { @@ -1642,6 +1682,22 @@ xfs_blockgc_free_quota( /* XFS Incore Inode Walking Code */ +static inline bool +xfs_grabbed_for_walk( + enum xfs_icwalk_goal goal, + struct xfs_inode *ip, + int iter_flags) +{ + switch (goal) { + case XFS_ICWALK_BLOCKGC: + return xfs_blockgc_igrab(ip, iter_flags); + case XFS_ICWALK_DQRELE: + return xfs_dqrele_igrab(ip); + default: + return false; + } +} + /* * For a given per-AG structure @pag, grab, @execute, and rele all incore * inodes with the given radix tree @tag. @@ -1695,7 +1751,7 @@ xfs_inode_walk_ag( for (i = 0; i < nr_found; i++) { struct xfs_inode *ip = batch[i]; - if (done || !xfs_inode_walk_ag_grab(ip, iter_flags)) + if (done || !xfs_grabbed_for_walk(goal, ip, iter_flags)) batch[i] = NULL; /*