Re: [PATCH bpf-next 1/2] mm, bpf: Introduce __GFP_TRYLOCK for opportunistic page allocation

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Sat, Nov 16, 2024 at 1:13 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Nov 16, 2024 at 11:42 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Fri, Nov 15, 2024 at 05:48:53PM -0800, Alexei Starovoitov wrote:
> > > +static inline struct page *try_alloc_page_noprof(int nid)
> > > +{
> > > +     /* If spin_locks are not held and interrupts are enabled, use normal path. */
> > > +     if (preemptible())
> > > +             return alloc_pages_node_noprof(nid, GFP_NOWAIT | __GFP_ZERO, 0);
> >
> > This isn't right for PREEMPT_RT, spinlock_t will be preemptible, but you
> > very much do not want regular allocation calls while inside the
> > allocator itself for example.
>
> I'm aware that spinlocks are preemptible in RT.
> Here is my understanding of why the above is correct...
> - preemptible() means that IRQs are not disabled and preempt_count == 0.
>
> - All page alloc operations are protected either by
> pcp_spin_trylock() or by spin_lock_irqsave(&zone->lock, flags)
> or both together.
>
> - In non-RT spin_lock_irqsave disables IRQs, so preemptible()
> check guarantees that we're not holding zone->lock.
> The page alloc logic can hold pcp lock when try_alloc_page() is called,
> but it's always using pcp_trylock, so it's still ok to call it
> with GFP_NOWAIT. pcp trylock will fail and zone->lock will proceed
> to acquire zone->lock.
>
> - In RT spin_lock_irqsave doesn't disable IRQs despite its name.
> It calls rt_spin_lock() which calls rcu_read_lock()
> which increments preempt_count.

The maze of ifdef-s beat me :(
It doesn't increment in PREEMPT_RCU.
Need an additional check then. hmm.