Skip to content

Commit 8dabe34

Browse files
ameryhungMartin KaFai Lau
authored andcommitted
bpf: Change local_storage->lock and b->lock to rqspinlock
Change bpf_local_storage::lock and bpf_local_storage_map_bucket::lock from raw_spin_lock to rqspinlock. Finally, propagate errors from raw_res_spin_lock_irqsave() to syscall return or BPF helper return. In bpf_local_storage_destroy(), ignore return from raw_res_spin_lock_irqsave() for now. A later patch will correctly handle errors correctly in bpf_local_storage_destroy() so that it can unlink selems even when failing to acquire locks. For __bpf_local_storage_map_cache(), instead of handling the error, skip updating the cache. Acked-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Amery Hung <ameryhung@gmail.com> Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Link: https://patch.msgid.link/20260205222916.1788211-6-ameryhung@gmail.com
1 parent 403e935 commit 8dabe34

2 files changed

Lines changed: 47 additions & 22 deletions

File tree

include/linux/bpf_local_storage.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,13 @@
1515
#include <linux/types.h>
1616
#include <linux/bpf_mem_alloc.h>
1717
#include <uapi/linux/btf.h>
18+
#include <asm/rqspinlock.h>
1819

1920
#define BPF_LOCAL_STORAGE_CACHE_SIZE 16
2021

2122
struct bpf_local_storage_map_bucket {
2223
struct hlist_head list;
23-
raw_spinlock_t lock;
24+
rqspinlock_t lock;
2425
};
2526

2627
/* Thp map is not the primary owner of a bpf_local_storage_elem.
@@ -94,7 +95,7 @@ struct bpf_local_storage {
9495
* bpf_local_storage_elem.
9596
*/
9697
struct rcu_head rcu;
97-
raw_spinlock_t lock; /* Protect adding/removing from the "list" */
98+
rqspinlock_t lock; /* Protect adding/removing from the "list" */
9899
bool use_kmalloc_nolock;
99100
};
100101

kernel/bpf/bpf_local_storage.c

Lines changed: 44 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -321,14 +321,18 @@ static int bpf_selem_unlink_map(struct bpf_local_storage_elem *selem)
321321
struct bpf_local_storage_map *smap;
322322
struct bpf_local_storage_map_bucket *b;
323323
unsigned long flags;
324+
int err;
324325

325326
local_storage = rcu_dereference_check(selem->local_storage,
326327
bpf_rcu_lock_held());
327328
smap = rcu_dereference_check(SDATA(selem)->smap, bpf_rcu_lock_held());
328329
b = select_bucket(smap, local_storage);
329-
raw_spin_lock_irqsave(&b->lock, flags);
330+
err = raw_res_spin_lock_irqsave(&b->lock, flags);
331+
if (err)
332+
return err;
333+
330334
hlist_del_init_rcu(&selem->map_node);
331-
raw_spin_unlock_irqrestore(&b->lock, flags);
335+
raw_res_spin_unlock_irqrestore(&b->lock, flags);
332336

333337
return 0;
334338
}
@@ -344,11 +348,16 @@ int bpf_selem_link_map(struct bpf_local_storage_map *smap,
344348
{
345349
struct bpf_local_storage_map_bucket *b;
346350
unsigned long flags;
351+
int err;
347352

348353
b = select_bucket(smap, local_storage);
349-
raw_spin_lock_irqsave(&b->lock, flags);
354+
355+
err = raw_res_spin_lock_irqsave(&b->lock, flags);
356+
if (err)
357+
return err;
358+
350359
hlist_add_head_rcu(&selem->map_node, &b->list);
351-
raw_spin_unlock_irqrestore(&b->lock, flags);
360+
raw_res_spin_unlock_irqrestore(&b->lock, flags);
352361

353362
return 0;
354363
}
@@ -365,7 +374,7 @@ int bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool reuse_now)
365374
bool free_local_storage = false;
366375
HLIST_HEAD(selem_free_list);
367376
unsigned long flags;
368-
int err = 0;
377+
int err;
369378

370379
if (unlikely(!selem_linked_to_storage_lockless(selem)))
371380
/* selem has already been unlinked from sk */
@@ -374,7 +383,10 @@ int bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool reuse_now)
374383
local_storage = rcu_dereference_check(selem->local_storage,
375384
bpf_rcu_lock_held());
376385

377-
raw_spin_lock_irqsave(&local_storage->lock, flags);
386+
err = raw_res_spin_lock_irqsave(&local_storage->lock, flags);
387+
if (err)
388+
return err;
389+
378390
if (likely(selem_linked_to_storage(selem))) {
379391
/* Always unlink from map before unlinking from local_storage
380392
* because selem will be freed after successfully unlinked from
@@ -388,7 +400,7 @@ int bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool reuse_now)
388400
local_storage, selem, &selem_free_list);
389401
}
390402
out:
391-
raw_spin_unlock_irqrestore(&local_storage->lock, flags);
403+
raw_res_spin_unlock_irqrestore(&local_storage->lock, flags);
392404

393405
bpf_selem_free_list(&selem_free_list, reuse_now);
394406

@@ -403,16 +415,20 @@ void __bpf_local_storage_insert_cache(struct bpf_local_storage *local_storage,
403415
struct bpf_local_storage_elem *selem)
404416
{
405417
unsigned long flags;
418+
int err;
406419

407420
/* spinlock is needed to avoid racing with the
408421
* parallel delete. Otherwise, publishing an already
409422
* deleted sdata to the cache will become a use-after-free
410423
* problem in the next bpf_local_storage_lookup().
411424
*/
412-
raw_spin_lock_irqsave(&local_storage->lock, flags);
425+
err = raw_res_spin_lock_irqsave(&local_storage->lock, flags);
426+
if (err)
427+
return;
428+
413429
if (selem_linked_to_storage(selem))
414430
rcu_assign_pointer(local_storage->cache[smap->cache_idx], SDATA(selem));
415-
raw_spin_unlock_irqrestore(&local_storage->lock, flags);
431+
raw_res_spin_unlock_irqrestore(&local_storage->lock, flags);
416432
}
417433

418434
static int check_flags(const struct bpf_local_storage_data *old_sdata,
@@ -457,14 +473,17 @@ int bpf_local_storage_alloc(void *owner,
457473

458474
RCU_INIT_POINTER(storage->smap, smap);
459475
INIT_HLIST_HEAD(&storage->list);
460-
raw_spin_lock_init(&storage->lock);
476+
raw_res_spin_lock_init(&storage->lock);
461477
storage->owner = owner;
462478
storage->use_kmalloc_nolock = smap->use_kmalloc_nolock;
463479

464480
bpf_selem_link_storage_nolock(storage, first_selem);
465481

466482
b = select_bucket(smap, storage);
467-
raw_spin_lock_irqsave(&b->lock, flags);
483+
err = raw_res_spin_lock_irqsave(&b->lock, flags);
484+
if (err)
485+
goto uncharge;
486+
468487
bpf_selem_link_map_nolock(b, first_selem);
469488

470489
owner_storage_ptr =
@@ -482,11 +501,11 @@ int bpf_local_storage_alloc(void *owner,
482501
prev_storage = cmpxchg(owner_storage_ptr, NULL, storage);
483502
if (unlikely(prev_storage)) {
484503
bpf_selem_unlink_map_nolock(first_selem);
485-
raw_spin_unlock_irqrestore(&b->lock, flags);
504+
raw_res_spin_unlock_irqrestore(&b->lock, flags);
486505
err = -EAGAIN;
487506
goto uncharge;
488507
}
489-
raw_spin_unlock_irqrestore(&b->lock, flags);
508+
raw_res_spin_unlock_irqrestore(&b->lock, flags);
490509

491510
return 0;
492511

@@ -569,7 +588,9 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
569588
if (!alloc_selem)
570589
return ERR_PTR(-ENOMEM);
571590

572-
raw_spin_lock_irqsave(&local_storage->lock, flags);
591+
err = raw_res_spin_lock_irqsave(&local_storage->lock, flags);
592+
if (err)
593+
goto free_selem;
573594

574595
/* Recheck local_storage->list under local_storage->lock */
575596
if (unlikely(hlist_empty(&local_storage->list))) {
@@ -596,7 +617,9 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
596617

597618
b = select_bucket(smap, local_storage);
598619

599-
raw_spin_lock_irqsave(&b->lock, b_flags);
620+
err = raw_res_spin_lock_irqsave(&b->lock, b_flags);
621+
if (err)
622+
goto unlock;
600623

601624
alloc_selem = NULL;
602625
/* First, link the new selem to the map */
@@ -612,9 +635,10 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
612635
&old_selem_free_list);
613636
}
614637

615-
raw_spin_unlock_irqrestore(&b->lock, b_flags);
638+
raw_res_spin_unlock_irqrestore(&b->lock, b_flags);
616639
unlock:
617-
raw_spin_unlock_irqrestore(&local_storage->lock, flags);
640+
raw_res_spin_unlock_irqrestore(&local_storage->lock, flags);
641+
free_selem:
618642
bpf_selem_free_list(&old_selem_free_list, false);
619643
if (alloc_selem) {
620644
mem_uncharge(smap, owner, smap->elem_size);
@@ -699,7 +723,7 @@ void bpf_local_storage_destroy(struct bpf_local_storage *local_storage)
699723
* when unlinking elem from the local_storage->list and
700724
* the map's bucket->list.
701725
*/
702-
raw_spin_lock_irqsave(&local_storage->lock, flags);
726+
raw_res_spin_lock_irqsave(&local_storage->lock, flags);
703727
hlist_for_each_entry_safe(selem, n, &local_storage->list, snode) {
704728
/* Always unlink from map before unlinking from
705729
* local_storage.
@@ -714,7 +738,7 @@ void bpf_local_storage_destroy(struct bpf_local_storage *local_storage)
714738
free_storage = bpf_selem_unlink_storage_nolock(
715739
local_storage, selem, &free_selem_list);
716740
}
717-
raw_spin_unlock_irqrestore(&local_storage->lock, flags);
741+
raw_res_spin_unlock_irqrestore(&local_storage->lock, flags);
718742

719743
bpf_selem_free_list(&free_selem_list, true);
720744

@@ -761,7 +785,7 @@ bpf_local_storage_map_alloc(union bpf_attr *attr,
761785

762786
for (i = 0; i < nbuckets; i++) {
763787
INIT_HLIST_HEAD(&smap->buckets[i].list);
764-
raw_spin_lock_init(&smap->buckets[i].lock);
788+
raw_res_spin_lock_init(&smap->buckets[i].lock);
765789
}
766790

767791
smap->elem_size = offsetof(struct bpf_local_storage_elem,

0 commit comments

Comments
 (0)