On Mon, Mar 29, 2021 at 09:06:25PM -0700, Darrick J. Wong wrote:

On Tue, Mar 30, 2021 at 02:07:47PM +1100, Dave Chinner wrote:

quoted

Next, igrab() takes a reference to the inode which will mark them
referenced. THis walk grabs every inode in the filesysetm cache,
so marks them all referenced and makes it harder to reclaim them
under memory pressure. This perturbs working set behaviour.

inode list walks and igrab/iput don't come for free - they perturb
the working set, LRU orders, cause lock contention, long tail
latencies, etc. The XFS inode cache walk might not be the prettiest
thing, but it doesn't have any of these nasty side effects.

So, in general, I don't think we should be adding new inode list
walks anywhere, not even deep in XFS where nobody else might care...

...but the current quotaoff behavior has /all/ of these problems too.

Yup, ISTR in the past it didn't even take inode references, but I
may be misremembering...

I think you and I hashed out on IRC that quotaoff could simply take the
ILOCK and the i_flags lock of every inode that isn't INEW, RECLAIMING,
or INACTIVATING; drop the dquots, and drop the locks, and then dqpurge
would only have to wait for the inodes that are actively being reclaimed
or inactivated.

I'm pretty sure that we only need to avoid INEW and IRECLAIM.
Anything else that has dquots attached needs to be locked and have
them removed. Those that are marked INEW will not have dquots
attached, and those marked IRECLAIM are in the process of being torn
down so their dquots will be released in short order.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com