On Thu, Jan 04, 2024 at 10:25:40AM +0100, Marco Elver wrote: > I think a boolean makes the interface more confusing for everyone > else. At that point stack_depot_put merely decrements the refcount and > becomes a wrapper around refcount_dec, right? Thanks Marco for the feedback. Fair enough. > I think you want to expose the stack_record struct anyway for your > series, so why not simply avoid calling stack_depot_put and decrement > the refcount with your own helper (there needs to be a new stackdepot > function to return a stack_record under the pool_rwlock held as > reader). Yeah, that was something I was experimenting with my last version. See [0], I moved the "stack_record" struct into the header so page_owner can make sense of it. I guess that's fine right? If so, I'd do as you mentioned, just decrementing it with my own helper so no calls to stack_depot_put will be needed. Regarding the locking, I yet have to check the patch that implements the read/write lock, but given that page_owner won't be evicting anything, do I still have to fiddle with the locks? > Also, you need to ensure noone else calls stack_depot_put on the stack > traces you want to keep. If there is a risk someone else may call > stack_depot_put on them, it obviously won't work (I think the only > option then is to introduce a way to pin stacks). Well, since page_owner won't call stack_depot_put, I don't see how someone else would be able to interfere there, so I think I am safe there. [0] https://patchwork.kernel.org/project/linux-mm/patch/20231120084300.4368-3-osalvador@xxxxxxx/ -- Oscar Salvador SUSE Labs
