Messages in this thread | | | Date | Tue, 6 Sep 2022 05:54:51 +0200 | From | Oscar Salvador <> | Subject | Re: [PATCH v2 1/3] lib/stackdepot: Add a refcount field in stack_record |
| |
On Mon, Sep 05, 2022 at 10:57:20PM +0200, Andrey Konovalov wrote: > On Mon, Sep 5, 2022 at 5:10 AM Oscar Salvador <osalvador@suse.de> wrote: > > +enum stack_depot_action { > > + STACK_DEPOT_ACTION_NONE, > > + STACK_DEPOT_ACTION_COUNT, > > +}; > > Hi Oscar,
Hi Andrey
> Why do we need these actions? Why not just increment the refcount on > each stack trace save?
Let me try to explain it.
Back in RFC, there were no actions and the refcount was incremented/decremented in __set_page_ownwer() and __reset_page_owner() functions.
This lead to a performance "problem", where you would look for the stack twice, one when save it and one when increment it.
We figured we could do better and, at least, for the __set_page_owner() we could look just once for the stacktrace when calling __stack_depot_save, and increment it directly there.
We cannot do that for __reset_page_owner(), because the stack we are saving is the freeing stacktrace, and we are not interested in that. That is why __reset_page_owner() does:
<--- depot_stack_handle_t alloc_handle;
... alloc_handle = page_owner->handle; handle = save_stack(GFP_NOWAIT | __GFP_NOWARN, STACK_DEPOT_ACTION_NONE); page_owner->free_handle = handle stack_depot_dec_count(alloc_handle); --->
alloc_handle contains the handle for the allocation stacktrace, which was set in __set_page_owner(), and page_owner->free handle contains the handle for the freeing stacktrace. But we are only interested in the allocation stack and we only want to increment/decrement that on allocation/free.
> Could you split out the stack depot and the page_owner changes into > separate patches?
I could, I am not sure if it would make the review any easier though, as you could not match stackdepot <-> page_owner changes.
And we should be adding a bunch of code that would not be used till later on. But I can try it out if there is a strong opinion.
thanks for your time!
-- Oscar Salvador SUSE Labs
| |