| Message ID | 172437085364.57482.4719664018853105804.stgit@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [01/26] xfs: define the on-disk format for the metadir feature | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Thu, Aug 22, 2024 at 05:05:01PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > 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@kernel.org> > --- > 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... -Dave.
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@kernel.org> > > > > 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@kernel.org> > > --- > > 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@fromorbit.com >
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; + ASSERT(ip->i_delayed_blks == 0); if (XFS_IS_REALTIME_INODE(ip)) { @@ -1754,6 +1761,8 @@ xfs_qm_vop_dqalloc( if (!XFS_IS_QUOTA_ON(mp)) return 0; + ASSERT(!xfs_is_metadir_inode(ip)); + lockflags = XFS_ILOCK_EXCL; xfs_ilock(ip, lockflags); @@ -1883,6 +1892,7 @@ xfs_qm_vop_chown( xfs_assert_ilocked(ip, XFS_ILOCK_EXCL); ASSERT(XFS_IS_QUOTA_ON(ip->i_mount)); + ASSERT(!xfs_is_metadir_inode(ip)); /* old dquot */ prevdq = *IO_olddq; @@ -1970,6 +1980,7 @@ xfs_qm_vop_create_dqattach( return; xfs_assert_ilocked(ip, XFS_ILOCK_EXCL); + ASSERT(!xfs_is_metadir_inode(ip)); if (udqp && XFS_IS_UQUOTA_ON(mp)) { ASSERT(ip->i_udquot == NULL); diff --git a/fs/xfs/xfs_quota.h b/fs/xfs/xfs_quota.h index 23d71a55bbc00..645761997bf2d 100644 --- a/fs/xfs/xfs_quota.h +++ b/fs/xfs/xfs_quota.h @@ -29,6 +29,11 @@ struct xfs_buf; (XFS_IS_GQUOTA_ON(mp) && (ip)->i_gdquot == NULL) || \ (XFS_IS_PQUOTA_ON(mp) && (ip)->i_pdquot == NULL)) +#define XFS_IS_DQDETACHED(ip) \ + ((ip)->i_udquot == NULL && \ + (ip)->i_gdquot == NULL && \ + (ip)->i_pdquot == NULL) + #define XFS_QM_NEED_QUOTACHECK(mp) \ ((XFS_IS_UQUOTA_ON(mp) && \ (mp->m_sb.sb_qflags & XFS_UQUOTA_CHKD) == 0) || \ diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c index b368e13424c4f..ca7df018290e0 100644 --- a/fs/xfs/xfs_trans_dquot.c +++ b/fs/xfs/xfs_trans_dquot.c @@ -156,6 +156,8 @@ xfs_trans_mod_ino_dquot( unsigned int field, int64_t delta) { + ASSERT(!xfs_is_metadir_inode(ip) || XFS_IS_DQDETACHED(ip)); + xfs_trans_mod_dquot(tp, dqp, field, delta); if (xfs_hooks_switched_on(&xfs_dqtrx_hooks_switch)) { @@ -247,6 +249,8 @@ xfs_trans_mod_dquot_byino( xfs_is_quota_inode(&mp->m_sb, ip->i_ino)) return; + ASSERT(!xfs_is_metadir_inode(ip) || XFS_IS_DQDETACHED(ip)); + if (XFS_IS_UQUOTA_ON(mp) && ip->i_udquot) xfs_trans_mod_ino_dquot(tp, ip, ip->i_udquot, field, delta); if (XFS_IS_GQUOTA_ON(mp) && ip->i_gdquot) @@ -962,6 +966,8 @@ xfs_trans_reserve_quota_nblks( if (!XFS_IS_QUOTA_ON(mp)) return 0; + if (xfs_is_metadir_inode(ip)) + return 0; ASSERT(!xfs_is_quota_inode(&mp->m_sb, ip->i_ino)); xfs_assert_ilocked(ip, XFS_ILOCK_EXCL);