On Fri, Jan 9, 2026 at 10:16 AM Martin KaFai Lau [off-list ref] wrote:

On 12/18/25 9:56 AM, Amery Hung wrote:

quoted

To prepare changing both bpf_local_storage_map_bucket::lock and
bpf_local_storage::lock to rqspinlock, convert bpf_selem_unlink() to
failable. It still always succeeds and returns 0 until the change
happens. No functional change.

For bpf_local_storage_map_free(), WARN_ON() for now as no real error
will happen until we switch to rqspinlock.

__must_check is added to the function declaration locally to make sure
all callers are accounted for during the conversion.

I don't see __must_check. The same for patch 2.

I only added it locally. I will follow your suggestion to include it
in the patchset.

Per your suggestion:
Ignore the warning instead of WARN_ON for now. Add __must_check to
functions when everything is ready.

quoted

Signed-off-by: Amery Hung <redacted>
---
  include/linux/bpf_local_storage.h | 2 +-
  kernel/bpf/bpf_cgrp_storage.c     | 3 +--
  kernel/bpf/bpf_inode_storage.c    | 4 +---
  kernel/bpf/bpf_local_storage.c    | 8 +++++---
  kernel/bpf/bpf_task_storage.c     | 4 +---
  net/core/bpf_sk_storage.c         | 4 +---
  6 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/include/linux/bpf_local_storage.h b/include/linux/bpf_local_storage.h
index 6cabf5154cf6..a94e12ddd83d 100644
--- a/include/linux/bpf_local_storage.h
+++ b/include/linux/bpf_local_storage.h

@@ -176,7 +176,7 @@ int bpf_local_storage_map_check_btf(const struct bpf_map *map,
  void bpf_selem_link_storage_nolock(struct bpf_local_storage *local_storage,
                                 struct bpf_local_storage_elem *selem);

-void bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool reuse_now);
+int bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool reuse_now);

  int bpf_selem_link_map(struct bpf_local_storage_map *smap,
                     struct bpf_local_storage_elem *selem);

diff --git a/kernel/bpf/bpf_cgrp_storage.c b/kernel/bpf/bpf_cgrp_storage.c
index 0687a760974a..8fef24fcac68 100644
--- a/kernel/bpf/bpf_cgrp_storage.c
+++ b/kernel/bpf/bpf_cgrp_storage.c

@@ -118,8 +118,7 @@ static int cgroup_storage_delete(struct cgroup *cgroup, struct bpf_map *map)
      if (!sdata)
              return -ENOENT;

-     bpf_selem_unlink(SELEM(sdata), false);
-     return 0;
+     return bpf_selem_unlink(SELEM(sdata), false);
  }

  static long bpf_cgrp_storage_delete_elem(struct bpf_map *map, void *key)

diff --git a/kernel/bpf/bpf_inode_storage.c b/kernel/bpf/bpf_inode_storage.c
index e54cce2b9175..cedc99184dad 100644
--- a/kernel/bpf/bpf_inode_storage.c
+++ b/kernel/bpf/bpf_inode_storage.c

@@ -110,9 +110,7 @@ static int inode_storage_delete(struct inode *inode, struct bpf_map *map)
      if (!sdata)
              return -ENOENT;

-     bpf_selem_unlink(SELEM(sdata), false);
-
-     return 0;
+     return bpf_selem_unlink(SELEM(sdata), false);
  }

  static long bpf_fd_inode_storage_delete_elem(struct bpf_map *map, void *key)

diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
index 0e3fa5fbaaf3..fa629a180e9e 100644
--- a/kernel/bpf/bpf_local_storage.c
+++ b/kernel/bpf/bpf_local_storage.c

@@ -367,7 +367,7 @@ static void bpf_selem_link_map_nolock(struct bpf_local_storage_map *smap,
      hlist_add_head_rcu(&selem->map_node, &b->list);
  }

-void bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool reuse_now)
+int bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool reuse_now)
  {
      struct bpf_local_storage *local_storage;
      bool free_local_storage = false;

@@ -377,7 +377,7 @@ void bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool reuse_now)

      if (unlikely(!selem_linked_to_storage_lockless(selem)))
              /* selem has already been unlinked from sk */
-             return;
+             return 0;

      local_storage = rcu_dereference_check(selem->local_storage,
                                            bpf_rcu_lock_held());

@@ -402,6 +402,8 @@ void bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool reuse_now)

      if (free_local_storage)
              bpf_local_storage_free(local_storage, reuse_now);
+
+     return err;

err is not used in patch 3 and then becomes useful in patch 4. The
ai-review discovered issue on err also. Squash patch 4 into patch 3. It
will be easier to read.

Got it. Will squash patch 3 and 4.

quoted

  }

  void __bpf_local_storage_insert_cache(struct bpf_local_storage *local_storage,

@@ -837,7 +839,7 @@ void bpf_local_storage_map_free(struct bpf_map *map,
                              struct bpf_local_storage_elem, map_node))) {
                      if (busy_counter)
                              this_cpu_inc(*busy_counter);
-                     bpf_selem_unlink(selem, true);
+                     WARN_ON(bpf_selem_unlink(selem, true));

nit. I would add __must_check to the needed functions in a single patch
when everything is ready instead of having an intermediate WARN_ON here
and then removed it later.