From: Michal Hocko <mhocko@suse.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
bpf@vger.kernel.org, andrii@kernel.org, memxor@gmail.com,
akpm@linux-foundation.org, vbabka@suse.cz, bigeasy@linutronix.de,
rostedt@goodmis.org, houtao1@huawei.com, hannes@cmpxchg.org,
shakeel.butt@linux.dev, willy@infradead.org, tglx@linutronix.de,
jannh@google.com, tj@kernel.org, linux-mm@kvack.org,
kernel-team@fb.com
Subject: Re: [PATCH bpf-next v4 1/6] mm, bpf: Introduce try_alloc_pages() for opportunistic page allocation
Date: Tue, 14 Jan 2025 11:43:08 +0100 [thread overview]
Message-ID: <Z4Y_vPMt2dO0-X-r@tiehlicka> (raw)
In-Reply-To: <20250114103946.GC8362@noisy.programming.kicks-ass.net>
On Tue 14-01-25 11:39:46, Peter Zijlstra wrote:
> On Tue, Jan 14, 2025 at 11:19:41AM +0100, Michal Hocko wrote:
> > On Tue 14-01-25 10:53:55, Peter Zijlstra wrote:
> > > On Mon, Jan 13, 2025 at 06:19:17PM -0800, Alexei Starovoitov wrote:
> > > > From: Alexei Starovoitov <ast@kernel.org>
> > > >
> > > > Tracing BPF programs execute from tracepoints and kprobes where
> > > > running context is unknown, but they need to request additional
> > > > memory.
> > >
> > > > The prior workarounds were using pre-allocated memory and
> > > > BPF specific freelists to satisfy such allocation requests.
> > > > Instead, introduce gfpflags_allow_spinning() condition that signals
> > > > to the allocator that running context is unknown.
> > > > Then rely on percpu free list of pages to allocate a page.
> > > > The rmqueue_pcplist() should be able to pop the page from.
> > > > If it fails (due to IRQ re-entrancy or list being empty) then
> > > > try_alloc_pages() attempts to spin_trylock zone->lock
> > > > and refill percpu freelist as normal.
> > >
> > > > BPF program may execute with IRQs disabled and zone->lock is
> > > > sleeping in RT, so trylock is the only option.
> > >
> > > how is spin_trylock() from IRQ context not utterly broken in RT?
> >
> > + if (IS_ENABLED(CONFIG_PREEMPT_RT) && (in_nmi() || in_hardirq()))
> > + return NULL;
> >
> > Deals with that, right?
>
> Changelog didn't really mention that, did it? -- it seems to imply quite
> the opposite :/
yes, one has to look into the implementation see all the constrains and
the changelog could/should be improved in that regards.
> But maybe, I suppose any BPF program needs to expect failure due to this
> being trylock. I just worry some programs will malfunction due to never
> succeeding -- and RT getting blamed for this.
This is a good question. The current implementation has fewer constrains
and there are no data points about the success rate with the new
implementation. But to be completely honest I am not really sure how
much BPF is used on PREEMPT_RT systems and with RT workloads so I am not
sure how much of a practical problem that is.
>
> Maybe I worry too much.
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2025-01-14 10:43 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-14 2:19 [PATCH bpf-next v4 0/6] bpf, mm: Introduce try_alloc_pages() Alexei Starovoitov
2025-01-14 2:19 ` [PATCH bpf-next v4 1/6] mm, bpf: Introduce try_alloc_pages() for opportunistic page allocation Alexei Starovoitov
2025-01-14 9:53 ` Peter Zijlstra
2025-01-14 10:19 ` Michal Hocko
2025-01-14 10:39 ` Peter Zijlstra
2025-01-14 10:43 ` Michal Hocko [this message]
2025-01-14 18:29 ` Alexei Starovoitov
2025-01-14 18:34 ` Steven Rostedt
2025-01-14 10:31 ` Michal Hocko
2025-01-15 1:23 ` Alexei Starovoitov
2025-01-15 8:35 ` Michal Hocko
2025-01-15 22:33 ` Alexei Starovoitov
2025-01-14 2:19 ` [PATCH bpf-next v4 2/6] mm, bpf: Introduce free_pages_nolock() Alexei Starovoitov
2025-01-14 2:19 ` [PATCH bpf-next v4 3/6] locking/local_lock: Introduce local_trylock_irqsave() Alexei Starovoitov
2025-01-14 2:19 ` [PATCH bpf-next v4 4/6] memcg: Use trylock to access memcg stock_lock Alexei Starovoitov
2025-01-14 10:39 ` Michal Hocko
2025-01-14 2:19 ` [PATCH bpf-next v4 5/6] mm, bpf: Use memcg in try_alloc_pages() Alexei Starovoitov
2025-01-14 2:19 ` [PATCH bpf-next v4 6/6] bpf: Use try_alloc_pages() to allocate pages for bpf needs Alexei Starovoitov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z4Y_vPMt2dO0-X-r@tiehlicka \
--to=mhocko@suse.com \
--cc=akpm@linux-foundation.org \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=bigeasy@linutronix.de \
--cc=bpf@vger.kernel.org \
--cc=hannes@cmpxchg.org \
--cc=houtao1@huawei.com \
--cc=jannh@google.com \
--cc=kernel-team@fb.com \
--cc=linux-mm@kvack.org \
--cc=memxor@gmail.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=shakeel.butt@linux.dev \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
--cc=vbabka@suse.cz \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.