On Tue, Mar 23, 2021 at 10:37:21AM +1100, Dave Chinner wrote: > On Tue, Mar 16, 2021 at 08:47:29AM -0700, Darrick J. Wong wrote: > > On Tue, Mar 16, 2021 at 07:27:10AM +0000, Christoph Hellwig wrote: > > > Still digesting this. What trips me off a bit is the huge amount of > > > duplication vs the inode reclaim mechanism. Did you look into sharing > > > more code there and if yes what speaks against that? > > > > TBH I didn't look /too/ hard because once upon a time[1] Dave was aiming > > to replace the inode reclaim tagging and iteration with an lru list walk > > so I decided not to entangle the two. > > > > [1] https://lore.kernel.org/linux-xfs/20191009032124.10541-23-david@xxxxxxxxxxxxx/ > > I prototyped that and discarded it - it made inode reclaim much, > much slower because it introduced delays (lock contention) adding > new inodes to the reclaim list while a reclaim isolation walk was in > progress. > > The radix tree based mechanism we have right now is very efficient > as only the inodes being marked for reclaim take the radix tree > lock and hence there is minimal contention for it... Ahah, that's what happened to that patchset. Well in that case, since xfs_reclaim_inodes* is going to stick around, I think it makes more sense to refactor xfs_inodes_walk_ag to handle XFS_ICI_RECLAIM_TAG, and then xfs_reclaim_inodes_ag can go away entirely. That said, xfs_reclaim_inodes_ag does have some warts (like updating the per-ag reclaim cursor and decrementing nr_to_scan) that would add clutter. --D > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx
