Re: [PATCH RFC 0/2] mm/page_alloc: free_pcppages_bulk safeguard

[Alexei Starovoitov]

On Tue, Aug 22, 2023 at 2:29 PM Chris Li <chrisl@xxxxxxxxxx> wrote:
>
> On Tue, Aug 22, 2023 at 2:19 PM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> > >
> > > The execution sequence is like this:
> > >
> > >        count = min(pcp->count, count);
> > >
> > >         /* Ensure requested pindex is drained first. */
> > >         pindex = pindex - 1;
> > >         bpf_injected_spin_lock_irqsave {
> > >                  alloc_page();
> > >                  original spin_lock_irqsave(&zone->lock, flags) ;
> > >         }
> >
> > bpf doesn't call into alloc_page() or slab alloc or pcpu alloc from
> > tracing progs.
> > All memory is preallocated.
>
> Here is the other patch submission thread which have more detail of
> how to reproduce it:
> https://lore.kernel.org/linux-mm/20230817-free_pcppages_bulk-v1-1-c14574a9f80c@xxxxxxxxxx/
>
> It is on older version of the kernel.

Please demonstrate the issue on the latest kernel.
It's an unnecessary time sink for everyone to review patches
targeting an issue in the old kernel.

> > Can you reproduce the issue on the latest upstream kernel?
>
> Hope, the fix on the BPF side went in as commit c66a36af7ba3a628.
> I am not aware of other cases.

That was a temporary workaround on perf side.
bpf task local storage was properly fixed later.

> It seems the consensus is so far is that we don't support BPF doing
> nested allocation on spin locks.
> That will implite any function called under the spinlocks as well.

We're still talking past each other. bpf uses preallocated memory.
It might look like bpf prog is allocating, but it's actually
not calling into slab.

> Do we care about adding more warnings on this kind of allocation at all?

bpf doesn't mess with mm state.
If you somehow managed to cause mm splat with bpf prog talk to bpf folks first.
It's a bug somewhere in bpf. Not with mm.