On Fri, Mar 19, 2021 at 06:25:01AM +0000, Christoph Hellwig wrote:
> On Thu, Mar 18, 2021 at 03:33:45PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > It turns out that there is a 1:1 mapping between the execute and tag
> > parameters that are passed to xfs_inode_walk_ag:
> >
> > xfs_dqrele_inode => XFS_ICI_NO_TAG
> > xfs_blockgc_scan_inode => XFS_ICI_BLOCKGC_TAG
> >
> > The radix tree tags are an implementation detail of the inode cache,
> > which means that callers outside of xfs_icache.c have no business
> > passing in radix tree tags. Since we're about to get rid of the
> > indirect calls in the BLOCKGC case, eliminate the extra argument in
> > favor of computing the ICI tag from the execute argument passed into the
> > function.
>
> This seems backwards to me. I'd rather deduce the function from the
> talk, which seems like a more sensible pattern.
>
> That being said, the quota inode walk is a little different in that it
> doesn't use any tags, so switching it to a plain list_for_each_entry_safe
> on sb->s_inodes would seems more sensible, something like this untested
> patch:
Something like this is probably going to work better (maybe split into
two patches). I'm going to kick off some testing on it:
---
>From 9ae07b6bf8c6b1337a627c8f0ad619c56511b343 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@xxxxxx>
Date: Fri, 19 Mar 2021 07:16:31 +0100
Subject: xfs: use s_inodes in xfs_qm_dqrele_all_inodes
Using xfs_inode_walk in xfs_qm_dqrele_all_inodes is complete overkill,
given that function simplify wants to iterate all live inodes known
to the VFS. Just iterate over the s_inodes list.
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
---
fs/xfs/xfs_qm_syscalls.c | 50 +++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/fs/xfs/xfs_qm_syscalls.c b/fs/xfs/xfs_qm_syscalls.c
index 11f1e2fbf22f44..4e33919ed04b56 100644
--- a/fs/xfs/xfs_qm_syscalls.c
+++ b/fs/xfs/xfs_qm_syscalls.c
@@ -748,41 +748,27 @@ xfs_qm_scall_getquota_next(
return error;
}
-STATIC int
+static void
xfs_dqrele_inode(
struct xfs_inode *ip,
- void *args)
+ uint flags)
{
- uint *flags = args;
-
- /* skip quota inodes */
- if (ip == ip->i_mount->m_quotainfo->qi_uquotaip ||
- ip == ip->i_mount->m_quotainfo->qi_gquotaip ||
- ip == ip->i_mount->m_quotainfo->qi_pquotaip) {
- ASSERT(ip->i_udquot == NULL);
- ASSERT(ip->i_gdquot == NULL);
- ASSERT(ip->i_pdquot == NULL);
- return 0;
- }
-
xfs_ilock(ip, XFS_ILOCK_EXCL);
- if ((*flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
+ if ((flags & XFS_UQUOTA_ACCT) && ip->i_udquot) {
xfs_qm_dqrele(ip->i_udquot);
ip->i_udquot = NULL;
}
- if ((*flags & XFS_GQUOTA_ACCT) && ip->i_gdquot) {
+ if ((flags & XFS_GQUOTA_ACCT) && ip->i_gdquot) {
xfs_qm_dqrele(ip->i_gdquot);
ip->i_gdquot = NULL;
}
- if ((*flags & XFS_PQUOTA_ACCT) && ip->i_pdquot) {
+ if ((flags & XFS_PQUOTA_ACCT) && ip->i_pdquot) {
xfs_qm_dqrele(ip->i_pdquot);
ip->i_pdquot = NULL;
}
xfs_iunlock(ip, XFS_ILOCK_EXCL);
- return 0;
}
-
/*
* Go thru all the inodes in the file system, releasing their dquots.
*
@@ -794,7 +780,29 @@ xfs_qm_dqrele_all_inodes(
struct xfs_mount *mp,
uint flags)
{
+ struct super_block *sb = mp->m_super;
+ struct inode *inode, *old_inode = NULL;
+
ASSERT(mp->m_quotainfo);
- xfs_inode_walk(mp, XFS_INODE_WALK_INEW_WAIT, xfs_dqrele_inode,
- &flags, XFS_ICI_NO_TAG);
+
+ spin_lock(&sb->s_inode_list_lock);
+ list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
+ struct xfs_inode *ip = XFS_I(inode);
+
+ if (xfs_is_quota_inode(&mp->m_sb, ip->i_ino))
+ continue;
+ if (!igrab(inode))
+ continue;
+ spin_unlock(&sb->s_inode_list_lock);
+
+ iput(old_inode);
+ old_inode = inode;
+
+ xfs_dqrele_inode(ip, flags);
+
+ spin_lock(&sb->s_inode_list_lock);
+ }
+ spin_unlock(&sb->s_inode_list_lock);
+
+ iput(old_inode);
}
--
2.30.1
