Skip to content

Commit c8be3da

Browse files
ameryhungMartin KaFai Lau
authored andcommitted
bpf: Prepare for bpf_selem_unlink_nofail()
The next patch will introduce bpf_selem_unlink_nofail() to handle rqspinlock errors. bpf_selem_unlink_nofail() will allow an selem to be partially unlinked from map or local storage. Save memory allocation method in selem so that later an selem can be correctly freed even when SDATA(selem)->smap is init to NULL. In addition, keep track of memory charge to the owner in local storage so that later bpf_selem_unlink_nofail() can return the correct memory charge to the owner. Updating local_storage->mem_charge is protected by local_storage->lock. Finally, extract miscellaneous tasks performed when unlinking an selem from local_storage into bpf_selem_unlink_storage_nolock_misc(). It will be reused by bpf_selem_unlink_nofail(). This patch also takes the chance to remove local_storage->smap, which is no longer used since commit f484f4a ("bpf: Replace bpf memory allocator with kmalloc_nolock() in local storage"). 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-10-ameryhung@gmail.com
1 parent 3417dff commit c8be3da

2 files changed

Lines changed: 37 additions & 37 deletions

File tree

include/linux/bpf_local_storage.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ struct bpf_local_storage_elem {
8080
* after raw_spin_unlock
8181
*/
8282
};
83-
/* 8 bytes hole */
83+
bool use_kmalloc_nolock;
84+
/* 7 bytes hole */
8485
/* The data is stored in another cacheline to minimize
8586
* the number of cachelines access during a cache hit.
8687
*/
@@ -89,13 +90,13 @@ struct bpf_local_storage_elem {
8990

9091
struct bpf_local_storage {
9192
struct bpf_local_storage_data __rcu *cache[BPF_LOCAL_STORAGE_CACHE_SIZE];
92-
struct bpf_local_storage_map __rcu *smap;
9393
struct hlist_head list; /* List of bpf_local_storage_elem */
9494
void *owner; /* The object that owns the above "list" of
9595
* bpf_local_storage_elem.
9696
*/
9797
struct rcu_head rcu;
9898
rqspinlock_t lock; /* Protect adding/removing from the "list" */
99+
u64 mem_charge; /* Copy of mem charged to owner. Protected by "lock" */
99100
bool use_kmalloc_nolock;
100101
};
101102

kernel/bpf/bpf_local_storage.c

Lines changed: 34 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ bpf_selem_alloc(struct bpf_local_storage_map *smap, void *owner,
8585

8686
if (selem) {
8787
RCU_INIT_POINTER(SDATA(selem)->smap, smap);
88+
selem->use_kmalloc_nolock = smap->use_kmalloc_nolock;
8889

8990
if (value) {
9091
/* No need to call check_and_init_map_value as memory is zero init */
@@ -214,7 +215,7 @@ void bpf_selem_free(struct bpf_local_storage_elem *selem,
214215

215216
smap = rcu_dereference_check(SDATA(selem)->smap, bpf_rcu_lock_held());
216217

217-
if (!smap->use_kmalloc_nolock) {
218+
if (!selem->use_kmalloc_nolock) {
218219
/*
219220
* No uptr will be unpin even when reuse_now == false since uptr
220221
* is only supported in task local storage, where
@@ -251,6 +252,30 @@ static void bpf_selem_free_list(struct hlist_head *list, bool reuse_now)
251252
bpf_selem_free(selem, reuse_now);
252253
}
253254

255+
static void bpf_selem_unlink_storage_nolock_misc(struct bpf_local_storage_elem *selem,
256+
struct bpf_local_storage_map *smap,
257+
struct bpf_local_storage *local_storage,
258+
bool free_local_storage)
259+
{
260+
void *owner = local_storage->owner;
261+
u32 uncharge = smap->elem_size;
262+
263+
if (rcu_access_pointer(local_storage->cache[smap->cache_idx]) ==
264+
SDATA(selem))
265+
RCU_INIT_POINTER(local_storage->cache[smap->cache_idx], NULL);
266+
267+
uncharge += free_local_storage ? sizeof(*local_storage) : 0;
268+
mem_uncharge(smap, local_storage->owner, uncharge);
269+
local_storage->mem_charge -= uncharge;
270+
271+
if (free_local_storage) {
272+
local_storage->owner = NULL;
273+
274+
/* After this RCU_INIT, owner may be freed and cannot be used */
275+
RCU_INIT_POINTER(*owner_storage(smap, owner), NULL);
276+
}
277+
}
278+
254279
/* local_storage->lock must be held and selem->local_storage == local_storage.
255280
* The caller must ensure selem->smap is still valid to be
256281
* dereferenced for its smap->elem_size and smap->cache_idx.
@@ -261,56 +286,30 @@ static bool bpf_selem_unlink_storage_nolock(struct bpf_local_storage *local_stor
261286
{
262287
struct bpf_local_storage_map *smap;
263288
bool free_local_storage;
264-
void *owner;
265289

266290
smap = rcu_dereference_check(SDATA(selem)->smap, bpf_rcu_lock_held());
267-
owner = local_storage->owner;
268-
269-
/* All uncharging on the owner must be done first.
270-
* The owner may be freed once the last selem is unlinked
271-
* from local_storage.
272-
*/
273-
mem_uncharge(smap, owner, smap->elem_size);
274291

275292
free_local_storage = hlist_is_singular_node(&selem->snode,
276293
&local_storage->list);
277-
if (free_local_storage) {
278-
mem_uncharge(smap, owner, sizeof(struct bpf_local_storage));
279-
local_storage->owner = NULL;
280294

281-
/* After this RCU_INIT, owner may be freed and cannot be used */
282-
RCU_INIT_POINTER(*owner_storage(smap, owner), NULL);
295+
bpf_selem_unlink_storage_nolock_misc(selem, smap, local_storage,
296+
free_local_storage);
283297

284-
/* local_storage is not freed now. local_storage->lock is
285-
* still held and raw_spin_unlock_bh(&local_storage->lock)
286-
* will be done by the caller.
287-
*
288-
* Although the unlock will be done under
289-
* rcu_read_lock(), it is more intuitive to
290-
* read if the freeing of the storage is done
291-
* after the raw_spin_unlock_bh(&local_storage->lock).
292-
*
293-
* Hence, a "bool free_local_storage" is returned
294-
* to the caller which then calls then frees the storage after
295-
* all the RCU grace periods have expired.
296-
*/
297-
}
298298
hlist_del_init_rcu(&selem->snode);
299-
if (rcu_access_pointer(local_storage->cache[smap->cache_idx]) ==
300-
SDATA(selem))
301-
RCU_INIT_POINTER(local_storage->cache[smap->cache_idx], NULL);
302299

303300
hlist_add_head(&selem->free_node, free_selem_list);
304301

305-
if (rcu_access_pointer(local_storage->smap) == smap)
306-
RCU_INIT_POINTER(local_storage->smap, NULL);
307-
308302
return free_local_storage;
309303
}
310304

311305
void bpf_selem_link_storage_nolock(struct bpf_local_storage *local_storage,
312306
struct bpf_local_storage_elem *selem)
313307
{
308+
struct bpf_local_storage_map *smap;
309+
310+
smap = rcu_dereference_check(SDATA(selem)->smap, bpf_rcu_lock_held());
311+
local_storage->mem_charge += smap->elem_size;
312+
314313
RCU_INIT_POINTER(selem->local_storage, local_storage);
315314
hlist_add_head_rcu(&selem->snode, &local_storage->list);
316315
}
@@ -471,10 +470,10 @@ int bpf_local_storage_alloc(void *owner,
471470
goto uncharge;
472471
}
473472

474-
RCU_INIT_POINTER(storage->smap, smap);
475473
INIT_HLIST_HEAD(&storage->list);
476474
raw_res_spin_lock_init(&storage->lock);
477475
storage->owner = owner;
476+
storage->mem_charge = sizeof(*storage);
478477
storage->use_kmalloc_nolock = smap->use_kmalloc_nolock;
479478

480479
bpf_selem_link_storage_nolock(storage, first_selem);

0 commit comments

Comments
 (0)