On Thu, Sep 07, 2023 at 05:11:53PM +1000, Dave Chinner wrote:
> On Tue, Sep 05, 2023 at 09:33:03AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Teach quotacheck to reload the unlinked inode lists when walking the
> > inode table. This requires extra state handling, since it's possible
> > that a reloaded inode will get inactivated before quotacheck tries to
> > scan it; in this case, we need to ensure that the reloaded inode does
> > not have dquots attached when it is freed.
> >
> > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > ---
> > v1.1: s/CONFIG_QUOTA/CONFIG_XFS_QUOTA/ and fix tracepoint flags decoding
> > ---
> > fs/xfs/xfs_inode.c | 12 +++++++++---
> > fs/xfs/xfs_inode.h | 5 ++++-
> > fs/xfs/xfs_mount.h | 10 +++++++++-
> > fs/xfs/xfs_qm.c | 7 +++++++
> > 4 files changed, 29 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index 56f6bde6001b..22af7268169b 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -1743,9 +1743,13 @@ xfs_inactive(
> > ip->i_df.if_nextents > 0 || ip->i_delayed_blks > 0))
> > truncate = 1;
> >
> > - error = xfs_qm_dqattach(ip);
> > - if (error)
> > - goto out;
> > + if (xfs_iflags_test(ip, XFS_IQUOTAUNCHECKED)) {
> > + xfs_qm_dqdetach(ip);
> > + } else {
> > + error = xfs_qm_dqattach(ip);
> > + if (error)
> > + goto out;
> > + }
>
> That needs a comment - I'm not going to remember why sometimes we
> detatch dquots instead of attach them here....
/*
* If this inode is being inactivated during a quotacheck and
* has not yet been scanned by quotacheck, we /must/ remove the
* dquots from the inode before inactivation changes the block
* and inode counts. Most probably this is a result of
* reloading the incore iunlinked list to purge unrecovered
* unlinked inodes.
*/
How does that sound?
> ....
> > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > index 6abcc34fafd8..7256090c3895 100644
> > --- a/fs/xfs/xfs_qm.c
> > +++ b/fs/xfs/xfs_qm.c
> > @@ -1160,6 +1160,10 @@ xfs_qm_dqusage_adjust(
> > if (error)
> > return error;
> >
> > + error = xfs_inode_reload_unlinked(ip);
> > + if (error)
> > + goto error0;
>
> Same comment here about doing millions of transaction create/cancel
> for inodes that have non-zero link counts....
>
> Also, same comment here about shutting down on reload error because
> the irele() call will inactivate the inode and try to remove it from
> the unlinked list....
Ok, fixed this callsite too.
Thank you for looking at this series!
--D
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
