On Wed, Jan 03, 2024 at 11:22:00PM -0800, Darrick J. Wong wrote: > On Thu, Jan 04, 2024 at 08:17:35AM +0100, Christoph Hellwig wrote: > > On Wed, Jan 03, 2024 at 11:14:54PM -0800, Darrick J. Wong wrote: > > > IIRC setting up the shrinker in xfs_alloc_buftarg_common takes some > > > shrinker lock somewhere, and lockdep complained about a potential > > > deadlock between the locks that scrub takes if I don't create the xfile > > > buftarg in the scrub _setup routines. That's why it's not created > > > internally to the xfbtree. > > > > > > I agree that it makes much more sense only to create those things when > > > they're actually needed, but ... hm. Maybe we don't need the xfile > > > buftarg to be hooked up to the shrinkers, seeing as it's ephemeral > > > anyway? That would save a lot of fuss and ... > > > > Yes, once we move to a model where the buffer always points to > > the shmem page, and we remove the buffer lru for them as we already > > have the page LRU there is no point in having a shrinker at all. > > Ok. I'll move the shrinker stuff into the real buftarg creation > function. This seems to have become a lot simpler now that the shrinker > is a pointer instead of embedded in the buftarg. Though looking at buftarg allocation and my old notes from a couple of years ago -- a second reason for allocating the buftarg during scrub setup was that the list_lru_init call allocates an array that's O(nodes_nr) and percpu_counter_init allocates an array that's O(maxcpus). At the time I decided that it was better to put those large contiguous memory allocations in the ->setup routine where we don't have any vfs/xfs locks held, can run direct reclaim, and haven't done any xfs work yet. So I'd like to leave the buftarg attached to struct xfs_scrub, though I'll still get rid of the shrinker for xfile buftargs. --D > > > > naming and moving it out of scrub/ would make sense as the concept > > > > isn't really scrub/repair specific. But if we want to stick with > > > > it I'd prefer to not also have _mem-based naming. > > > > > > Yes, lets move it to libxfs/xfbtree.[ch]. > > > > What does the xf in the various scrubx/xf* thinks stand for, btw? > > Why not libxfs/xfs_btree_mem.[ch]? > > "xfile btree". > > --D >
