All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin KaFai Lau <martin.lau@linux.dev>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Amery Hung <ameryhung@gmail.com>, bpf <bpf@vger.kernel.org>,
	Network Development <netdev@vger.kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@kernel.org>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	Song Liu <song@kernel.org>, Kernel Team <kernel-team@meta.com>
Subject: Re: [PATCH v2 bpf-next 4/4] bpf: Replace bpf memory allocator with kmalloc_nolock() in local storage
Date: Mon, 17 Nov 2025 11:21:57 -0800	[thread overview]
Message-ID: <ebbf8915-1404-4d4f-9b5a-b2f3924ec43a@linux.dev> (raw)
In-Reply-To: <CAADnVQJD0xLa=bWUerdYsRg8R4S54yqnPnuwkHWL1R663U3Xcg@mail.gmail.com>

On 11/14/25 6:01 PM, Alexei Starovoitov wrote:
> On Fri, Nov 14, 2025 at 12:13 PM Amery Hung <ameryhung@gmail.com> wrote:
>>
>>
>> -       if (smap->bpf_ma) {
>> +       if (smap->use_kmalloc_nolock) {
>>                  rcu_barrier_tasks_trace();
>> -               if (!rcu_trace_implies_rcu_gp())
>> -                       rcu_barrier();
>> -               bpf_mem_alloc_destroy(&smap->selem_ma);
>> -               bpf_mem_alloc_destroy(&smap->storage_ma);
>> +               rcu_barrier();
> 
> Why unconditional rcu_barrier() ?
> It's implied in rcu_barrier_tasks_trace().
> What am I missing?

Amery probably can confirm. I think the bpf_obj_free_fields() may only need to
wait for a rcu gp without going through a rcu_tasks_trace gp and the tasks_trace
cb, so it needs to ensure all rcu callbacks has finished.

@@ -247,18 +231,11 @@ void bpf_selem_free(struct bpf_local_storage_elem *selem,
  	}
  
  	if (reuse_now) {
-		/* reuse_now == true only happens when the storage owner
-		 * (e.g. task_struct) is being destructed or the map itself
-		 * is being destructed (ie map_free). In both cases,
-		 * no bpf prog can have a hold on the selem. It is
-		 * safe to unpin the uptrs and free the selem now.
-		 */
-		bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
-		/* Instead of using the vanilla call_rcu(),
-		 * bpf_mem_cache_free will be able to reuse selem
-		 * immediately.
+		/*
+		 * While it is okay to call bpf_obj_free_fields() that unpins uptr when
+		 * reuse_now == true, keep it in bpf_selem_free_rcu() for simplicity.
  		 */
-		bpf_mem_cache_free(&smap->selem_ma, selem);
+		call_rcu(&selem->rcu, bpf_selem_free_rcu);
  		return;
  	}


Others lgtm also,

Reviewed-by: Martin KaFai Lau <martin.lau@kernel.org>

  reply	other threads:[~2025-11-17 19:22 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-14 20:13 [PATCH v2 bpf-next 0/4] Replace BPF memory allocator with kmalloc_nolock() in local storage Amery Hung
2025-11-14 20:13 ` [PATCH v2 bpf-next 1/4] bpf: Always charge/uncharge memory when allocating/unlinking storage elements Amery Hung
2025-11-17 18:25   ` Martin KaFai Lau
2025-11-14 20:13 ` [PATCH v2 bpf-next 2/4] bpf: Remove smap argument from bpf_selem_free() Amery Hung
2025-11-17 18:32   ` Martin KaFai Lau
2025-11-14 20:13 ` [PATCH v2 bpf-next 3/4] bpf: Save memory alloction info in bpf_local_storage Amery Hung
2025-11-17 18:36   ` Martin KaFai Lau
2025-11-14 20:13 ` [PATCH v2 bpf-next 4/4] bpf: Replace bpf memory allocator with kmalloc_nolock() in local storage Amery Hung
2025-11-15  2:01   ` Alexei Starovoitov
2025-11-17 19:21     ` Martin KaFai Lau [this message]
2025-11-17 20:37     ` Amery Hung
2025-11-17 23:36       ` Alexei Starovoitov
2025-11-17 23:46         ` Paul E. McKenney
2025-11-18  0:24           ` Amery Hung
2025-11-18  0:41             ` Paul E. McKenney
2026-04-10  1:43   ` Slava Imameev
2025-11-19  0:30 ` [PATCH v2 bpf-next 0/4] Replace BPF " patchwork-bot+netdevbpf

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=ebbf8915-1404-4d4f-9b5a-b2f3924ec43a@linux.dev \
    --to=martin.lau@linux.dev \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ameryhung@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@meta.com \
    --cc=kpsingh@kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=memxor@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /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.