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: > > 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@xxxxxxxxxxxxx
