On Mon, Aug 26, 2024 at 10:47:41AM +1000, Dave Chinner wrote:
> On Thu, Aug 22, 2024 at 05:05:01PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > Files in the metadata directory tree are internal to the filesystem.
> > Don't count the inodes or the blocks they use in the root dquot because
> > users do not need to know about their resource usage. This will also
> > quiet down complaints about dquot usage not matching du output.
> >
> > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_dquot.c | 1 +
> > fs/xfs/xfs_qm.c | 11 +++++++++++
> > fs/xfs/xfs_quota.h | 5 +++++
> > fs/xfs/xfs_trans_dquot.c | 6 ++++++
> > 4 files changed, 23 insertions(+)
> >
> >
> > diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> > index c1b211c260a9d..3bf47458c517a 100644
> > --- a/fs/xfs/xfs_dquot.c
> > +++ b/fs/xfs/xfs_dquot.c
> > @@ -983,6 +983,7 @@ xfs_qm_dqget_inode(
> >
> > xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
> > ASSERT(xfs_inode_dquot(ip, type) == NULL);
> > + ASSERT(!xfs_is_metadir_inode(ip));
> >
> > id = xfs_qm_id_for_quotatype(ip, type);
> >
> > diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> > index d0674d84af3ec..ec983cca9adae 100644
> > --- a/fs/xfs/xfs_qm.c
> > +++ b/fs/xfs/xfs_qm.c
> > @@ -304,6 +304,8 @@ xfs_qm_need_dqattach(
> > return false;
> > if (xfs_is_quota_inode(&mp->m_sb, ip->i_ino))
> > return false;
> > + if (xfs_is_metadir_inode(ip))
> > + return false;
> > return true;
> > }
> >
> > @@ -326,6 +328,7 @@ xfs_qm_dqattach_locked(
> > return 0;
> >
> > xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);
> > + ASSERT(!xfs_is_metadir_inode(ip));
> >
> > if (XFS_IS_UQUOTA_ON(mp) && !ip->i_udquot) {
> > error = xfs_qm_dqattach_one(ip, XFS_DQTYPE_USER,
> > @@ -1204,6 +1207,10 @@ xfs_qm_dqusage_adjust(
> > }
> > }
> >
> > + /* Metadata directory files are not accounted to user-visible quotas. */
> > + if (xfs_is_metadir_inode(ip))
> > + goto error0;
> > +
>
> Hmmmm. I'm starting to think that xfs_iget() should not return
> metadata inodes unless a new XFS_IGET_METAINODE flag is set.
>
> That would replace all these post xfs_iget() checks with a single
> check in xfs_iget(), and then xfs_trans_metafile_iget() is the only
> place that sets this specific flag.
>
> That means stuff like VFS lookups, bulkstat, quotacheck, and
> filehandle lookups will never return metadata inodes and we don't
> need to add special checks all over for them...
I think doing so is likely an overall improvement for the codebase, but
it will complicate life for the directory and parent pointer scrubbers
because they can be called with the ino/gen of files in metadata
directory tree, and there's a special bulkstat mode for xfs_scrub
wherein one can get the ino/gen of metadata directory tree files.
Anyway I'll think further about this as a separate cleanup.
Note that the checks in this particular patch exist so that we don't
have to sprinkle them all over the bmap code so that the rtbitmap and
summary files can be excluded from quota accounting.
--D
>
> -Dave.
> --
> Dave Chinner
> david@xxxxxxxxxxxxx
>
