Hi Michal,
On 8/16/2022 9:45 PM, Michal Hocko wrote:
>>>> @@ -183,19 +184,26 @@ static inline void __set_page_owner_handle(struct page_ext *page_ext,
>>>> noinline void __set_page_owner(struct page *page, unsigned short order,
>>>> gfp_t gfp_mask)
>>>> {
>>>> - struct page_ext *page_ext = lookup_page_ext(page);
>>>> + struct page_ext *page_ext = page_ext_get(page);
>>>> depot_stack_handle_t handle;
>>>>
>>>> if (unlikely(!page_ext))
>>>> return;
>>> Either add a comment like this
>>> /* save_stack can sleep in general so we have to page_ext_put */
>>
>> Vlastimil suggested to go for save stack first since !page_ext is mostly
>> unlikely. Snip from his comments:
>> Why not simply do the save_stack() first and then page_ext_get() just
>> once? It should be really rare that it's NULL, so I don't think we save
>> much by avoiding an unnecessary save_stack(), while the overhead of
>> doing two get/put instead of one will affect every call.
> right see below
>> https://lore.kernel.org/all/f5fd4942-b03e-1d1c-213b-9cd5283ced91@xxxxxxx/
>>>> + page_ext_put();
>>>>
>>>> handle = save_stack(gfp_mask);
>>> or just drop the initial page_ext_get altogether. This function is
>>> called only when page_ext is supposed to be initialized and !page_ext
>>> case above should be very unlikely. Or is there any reason to keep this?
I don't think that !page_ext check is really required as
__set_page_owner() is called means page_ext should have been
initialized. Will raise a separate change for this suggestion. For now
V4 is raised with the earlier suggestion of dropping the initial
page_ext.
https://lore.kernel.org/all/1660830600-9068-1-git-send-email-quic_charante@xxxxxxxxxxx/.
Thanks,
Charan
