From: Martin KaFai Lau <martin.lau@linux.dev>
To: Amery Hung <ameryhung@gmail.com>
Cc: netdev@vger.kernel.org, alexei.starovoitov@gmail.com,
andrii@kernel.org, daniel@iogearbox.net, memxor@gmail.com,
martin.lau@kernel.org, kpsingh@kernel.org,
yonghong.song@linux.dev, song@kernel.org, haoluo@google.com,
kernel-team@meta.com, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v5 10/16] bpf: Support lockless unlink when freeing map or local storage
Date: Wed, 4 Feb 2026 17:08:23 -0800 [thread overview]
Message-ID: <4c125283-b5a4-47f2-be84-a932b50312ab@linux.dev> (raw)
In-Reply-To: <CAMB2axPMJe6hGeyaAXvzHKyap+9uoH=q66dtCTD-C4zioJa5DA@mail.gmail.com>
On 2/4/26 3:14 PM, Amery Hung wrote:
> On Tue, Feb 3, 2026 at 9:39 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>>
>> On 2/1/26 9:50 AM, Amery Hung wrote:
>>> +/*
>>> + * Unlink an selem from map and local storage with lockless fallback if callers
>>> + * are racing or rqspinlock returns error. It should only be called by
>>> + * bpf_local_storage_destroy() or bpf_local_storage_map_free().
>>> + */
>>> +static void bpf_selem_unlink_nofail(struct bpf_local_storage_elem *selem,
>>> + struct bpf_local_storage_map_bucket *b)
>>> +{
>>> + struct bpf_local_storage *local_storage;
>>> + struct bpf_local_storage_map *smap;
>>> + bool in_map_free = !!b;
>>> + unsigned long flags;
>>> + int err, unlink = 0;
>>> +
>>> + local_storage = rcu_dereference_check(selem->local_storage, bpf_rcu_lock_held());
>>> + smap = rcu_dereference_check(SDATA(selem)->smap, bpf_rcu_lock_held());
>>> +
>>> + /*
>>> + * Prevent being called twice from the same caller on the same selem.
>>> + * map_free() and destroy() each holds a link_cnt on an selem.
>>> + */
>>> + if ((!smap && in_map_free) || (!local_storage && !in_map_free))
>>> + return;
>>> +
>>> + if (smap) {
>>> + b = b ? : select_bucket(smap, local_storage);
>>> + err = raw_res_spin_lock_irqsave(&b->lock, flags);
>>> + if (!err) {
>>> + /*
>>> + * Call bpf_obj_free_fields() under b->lock to make sure it is done
>>> + * exactly once for an selem. Safe to free special fields immediately
>>> + * as no BPF program should be referencing the selem.
>>> + */
>>> + if (likely(selem_linked_to_map(selem))) {
>>> + hlist_del_init_rcu(&selem->map_node);
>>> + bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
>>> + unlink++;
>>> + }
>>> + raw_res_spin_unlock_irqrestore(&b->lock, flags);
>>> + }
>>> + /*
>>> + * Highly unlikely scenario: resource leak
>>> + *
>>> + * When map_free(selem1), destroy(selem1) and destroy(selem2) are racing
>>> + * and both selem belong to the same bucket, if destroy(selem2) acquired
>>> + * b->lock and block for too long, neither map_free(selem1) and
>>> + * destroy(selem1) will be able to free the special field associated
>>> + * with selem1 as raw_res_spin_lock_irqsave() returns -ETIMEDOUT.
>>> + */
>>> + WARN_ON_ONCE(err && in_map_free);
>>> + if (!err || in_map_free)
>>> + RCU_INIT_POINTER(SDATA(selem)->smap, NULL);
>>> + }
>>> +
>>> + if (local_storage) {
>>> + err = raw_res_spin_lock_irqsave(&local_storage->lock, flags);
>>> + if (!err) {
>>> + /*
>>> + * Normally, map_free() can call mem_uncharge() if destroy() is
>>> + * not about to return to the owner, which can then go away
>>> + * immediately. Otherwise, the charge of the selem will stay
>>> + * accounted in local_storage->selems_size and uncharged during
>>> + * destroy().
>>> + */
>>> + if (likely(selem_linked_to_storage(selem))) {
>>> + hlist_del_init_rcu(&selem->snode);
>>> + if (smap && in_map_free &&
>>
>> I think the smap non-null check is not needed.
>
> While smap is still valid in map_free(), SDATA(selem)->smap could have
> been init to NULL, and then mem_uncharge() will dereference a null
> pointer.
hmm... there is a "if ((!smap && in_map_free) || ...)) return;" at the
beginning of the function, but may be the next revision will need this
check though if it does not depend on "!smap" to decide the second visit.
>>
>>
next prev parent reply other threads:[~2026-02-05 1:08 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-01 17:50 [PATCH bpf-next v5 00/16] Remove task and cgroup local storage percpu counters Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 01/16] bpf: Select bpf_local_storage_map_bucket based on bpf_local_storage Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 02/16] bpf: Convert bpf_selem_unlink_map to failable Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 03/16] bpf: Convert bpf_selem_link_map " Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 04/16] bpf: Convert bpf_selem_unlink " Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 05/16] bpf: Change local_storage->lock and b->lock to rqspinlock Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 06/16] bpf: Remove task local storage percpu counter Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 07/16] bpf: Remove cgroup " Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 08/16] bpf: Remove unused percpu counter from bpf_local_storage_map_free Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 09/16] bpf: Prepare for bpf_selem_unlink_nofail() Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 10/16] bpf: Support lockless unlink when freeing map or local storage Amery Hung
2026-02-01 18:22 ` bot+bpf-ci
2026-02-04 5:39 ` Martin KaFai Lau
2026-02-04 23:14 ` Amery Hung
2026-02-05 1:08 ` Martin KaFai Lau [this message]
2026-02-01 17:50 ` [PATCH bpf-next v5 11/16] bpf: Switch to bpf_selem_unlink_nofail in bpf_local_storage_{map_free, destroy} Amery Hung
2026-02-04 1:52 ` Martin KaFai Lau
2026-02-04 23:20 ` Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 12/16] selftests/bpf: Update sk_storage_omem_uncharge test Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 13/16] selftests/bpf: Update task_local_storage/recursion test Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 14/16] selftests/bpf: Update task_local_storage/task_storage_nodeadlock test Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 15/16] selftests/bpf: Remove test_task_storage_map_stress_lookup Amery Hung
2026-02-01 17:50 ` [PATCH bpf-next v5 16/16] selftests/bpf: Choose another percpu variable in bpf for btf_dump test Amery Hung
2026-02-01 23:29 ` [PATCH bpf-next v5 00/16] Remove task and cgroup local storage percpu counters 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=4c125283-b5a4-47f2-be84-a932b50312ab@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=haoluo@google.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox