Re: [RFC-PATCH 2/4] mm: Add __rcu_alloc_page_lockless() func.

[Vlastimil Babka <vbabka@suse.cz>]

On 9/30/20 12:07 AM, Uladzislau Rezki wrote:
> On Tue, Sep 29, 2020 at 12:15:34PM +0200, Vlastimil Babka wrote:
>> On 9/18/20 9:48 PM, Uladzislau Rezki (Sony) wrote:
>> 
>> After reading all the threads and mulling over this, I am going to deflect from
>> Mel and Michal and not oppose the idea of lockless allocation. I would even
>> prefer to do it via the gfp flag and not a completely separate path. Not using
>> the exact code from v1, I think it could be done in a way that we don't actually
>> look at the new flag until we find that pcplist is empty - which should not
>> introduce overhead to the fast-fast path when pcpclist is not empty. It's more
>> maintainable that adding new entry points, IMHO.
>> 
> Thanks for reading all that from the beginning! It must be tough due to the
> fact there were lot of messages in the threads, so at least i was lost.
> 
> I agree that adding a new entry or separate lock-less function can be considered
> as something that is hard to maintain. I have a question here. I mean about your
> different look at it:
> 
> <snip>
> bool is_pcp_cache_empty(gfp_t gfp)
> {
>     struct per_cpu_pages *pcp;
>     struct zoneref *ref;
>     unsigned long flags;
>     bool empty;
> 
>     ref = first_zones_zonelist(node_zonelist(
>             numa_node_id(), gfp), gfp_zone(gfp), NULL);
>     if (!ref->zone)
>             return true;
> 
>     local_irq_save(flags);
>     pcp = &this_cpu_ptr(ref->zone->pageset)->pcp;
>     empty = list_empty(&pcp->lists[gfp_migratetype(gfp)]);
>     local_irq_restore(flags);
> 
>     return empty;
> }
> 
> disable_irq();
> if (!is_pcp_cache_empty(GFP_NOWAIT))
>     __get_free_page(GFP_NOWAIT);
> enable_irq();
> <snip>
> 
> Do you mean to have something like above? I mean some extra API
> function that returns true or false if fast-fast allocation can
> either occur or not. Above code works just fine and never touches
> main zone->lock.
> 
> i.e. Instead of introducing an extra GFP_LOCKLESS flag or any new
> extra lock-less function. We could have something that checks a
> pcp page cache list, thus it can guarantee that a request would
> be accomplish using fast-fast path.

No, I meant going back to idea of new gfp flag, but adjust the implementation in
the allocator (different from what you posted in previous version) so that it
only looks at the flag after it tries to allocate from pcplist and finds out
it's empty. So, no inventing of new page allocator entry points or checks such
as the one you wrote above, but adding the new gfp flag in a way that it doesn't
affect existing fast paths.