Skip to content

Commit dcf456c

Browse files
sinkapAlexei Starovoitov
authored andcommitted
bpf: Fix usage of trace RCU in local storage.
bpf_{sk,task,inode}_storage_free() do not need to use call_rcu_tasks_trace as no BPF program should be accessing the owner as it's being destroyed. The only other reader at this point is bpf_local_storage_map_free() which uses normal RCU. The only path that needs trace RCU are: * bpf_local_storage_{delete,update} helpers * map_{delete,update}_elem() syscalls Fixes: 0fe4b38 ("bpf: Allow bpf_local_storage to be used by sleepable programs") Signed-off-by: KP Singh <kpsingh@kernel.org> Signed-off-by: Alexei Starovoitov <ast@kernel.org> Acked-by: Martin KaFai Lau <kafai@fb.com> Link: https://lore.kernel.org/bpf/20220418155158.2865678-1-kpsingh@kernel.org
1 parent 0e5aefa commit dcf456c

File tree

5 files changed

+28
-19
lines changed

5 files changed

+28
-19
lines changed

include/linux/bpf_local_storage.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ void bpf_selem_link_storage_nolock(struct bpf_local_storage *local_storage,
143143

144144
bool bpf_selem_unlink_storage_nolock(struct bpf_local_storage *local_storage,
145145
struct bpf_local_storage_elem *selem,
146-
bool uncharge_omem);
146+
bool uncharge_omem, bool use_trace_rcu);
147147

148-
void bpf_selem_unlink(struct bpf_local_storage_elem *selem);
148+
void bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool use_trace_rcu);
149149

150150
void bpf_selem_link_map(struct bpf_local_storage_map *smap,
151151
struct bpf_local_storage_elem *selem);

kernel/bpf/bpf_inode_storage.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ void bpf_inode_storage_free(struct inode *inode)
9090
*/
9191
bpf_selem_unlink_map(selem);
9292
free_inode_storage = bpf_selem_unlink_storage_nolock(
93-
local_storage, selem, false);
93+
local_storage, selem, false, false);
9494
}
9595
raw_spin_unlock_bh(&local_storage->lock);
9696
rcu_read_unlock();
@@ -149,7 +149,7 @@ static int inode_storage_delete(struct inode *inode, struct bpf_map *map)
149149
if (!sdata)
150150
return -ENOENT;
151151

152-
bpf_selem_unlink(SELEM(sdata));
152+
bpf_selem_unlink(SELEM(sdata), true);
153153

154154
return 0;
155155
}

kernel/bpf/bpf_local_storage.c

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ static void bpf_selem_free_rcu(struct rcu_head *rcu)
106106
*/
107107
bool bpf_selem_unlink_storage_nolock(struct bpf_local_storage *local_storage,
108108
struct bpf_local_storage_elem *selem,
109-
bool uncharge_mem)
109+
bool uncharge_mem, bool use_trace_rcu)
110110
{
111111
struct bpf_local_storage_map *smap;
112112
bool free_local_storage;
@@ -150,11 +150,16 @@ bool bpf_selem_unlink_storage_nolock(struct bpf_local_storage *local_storage,
150150
SDATA(selem))
151151
RCU_INIT_POINTER(local_storage->cache[smap->cache_idx], NULL);
152152

153-
call_rcu_tasks_trace(&selem->rcu, bpf_selem_free_rcu);
153+
if (use_trace_rcu)
154+
call_rcu_tasks_trace(&selem->rcu, bpf_selem_free_rcu);
155+
else
156+
kfree_rcu(selem, rcu);
157+
154158
return free_local_storage;
155159
}
156160

157-
static void __bpf_selem_unlink_storage(struct bpf_local_storage_elem *selem)
161+
static void __bpf_selem_unlink_storage(struct bpf_local_storage_elem *selem,
162+
bool use_trace_rcu)
158163
{
159164
struct bpf_local_storage *local_storage;
160165
bool free_local_storage = false;
@@ -169,12 +174,16 @@ static void __bpf_selem_unlink_storage(struct bpf_local_storage_elem *selem)
169174
raw_spin_lock_irqsave(&local_storage->lock, flags);
170175
if (likely(selem_linked_to_storage(selem)))
171176
free_local_storage = bpf_selem_unlink_storage_nolock(
172-
local_storage, selem, true);
177+
local_storage, selem, true, use_trace_rcu);
173178
raw_spin_unlock_irqrestore(&local_storage->lock, flags);
174179

175-
if (free_local_storage)
176-
call_rcu_tasks_trace(&local_storage->rcu,
180+
if (free_local_storage) {
181+
if (use_trace_rcu)
182+
call_rcu_tasks_trace(&local_storage->rcu,
177183
bpf_local_storage_free_rcu);
184+
else
185+
kfree_rcu(local_storage, rcu);
186+
}
178187
}
179188

180189
void bpf_selem_link_storage_nolock(struct bpf_local_storage *local_storage,
@@ -214,14 +223,14 @@ void bpf_selem_link_map(struct bpf_local_storage_map *smap,
214223
raw_spin_unlock_irqrestore(&b->lock, flags);
215224
}
216225

217-
void bpf_selem_unlink(struct bpf_local_storage_elem *selem)
226+
void bpf_selem_unlink(struct bpf_local_storage_elem *selem, bool use_trace_rcu)
218227
{
219228
/* Always unlink from map before unlinking from local_storage
220229
* because selem will be freed after successfully unlinked from
221230
* the local_storage.
222231
*/
223232
bpf_selem_unlink_map(selem);
224-
__bpf_selem_unlink_storage(selem);
233+
__bpf_selem_unlink_storage(selem, use_trace_rcu);
225234
}
226235

227236
struct bpf_local_storage_data *
@@ -466,7 +475,7 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
466475
if (old_sdata) {
467476
bpf_selem_unlink_map(SELEM(old_sdata));
468477
bpf_selem_unlink_storage_nolock(local_storage, SELEM(old_sdata),
469-
false);
478+
false, true);
470479
}
471480

472481
unlock:
@@ -548,7 +557,7 @@ void bpf_local_storage_map_free(struct bpf_local_storage_map *smap,
548557
migrate_disable();
549558
__this_cpu_inc(*busy_counter);
550559
}
551-
bpf_selem_unlink(selem);
560+
bpf_selem_unlink(selem, false);
552561
if (busy_counter) {
553562
__this_cpu_dec(*busy_counter);
554563
migrate_enable();

kernel/bpf/bpf_task_storage.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ void bpf_task_storage_free(struct task_struct *task)
102102
*/
103103
bpf_selem_unlink_map(selem);
104104
free_task_storage = bpf_selem_unlink_storage_nolock(
105-
local_storage, selem, false);
105+
local_storage, selem, false, false);
106106
}
107107
raw_spin_unlock_irqrestore(&local_storage->lock, flags);
108108
bpf_task_storage_unlock();
@@ -192,7 +192,7 @@ static int task_storage_delete(struct task_struct *task, struct bpf_map *map)
192192
if (!sdata)
193193
return -ENOENT;
194194

195-
bpf_selem_unlink(SELEM(sdata));
195+
bpf_selem_unlink(SELEM(sdata), true);
196196

197197
return 0;
198198
}

net/core/bpf_sk_storage.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ static int bpf_sk_storage_del(struct sock *sk, struct bpf_map *map)
4040
if (!sdata)
4141
return -ENOENT;
4242

43-
bpf_selem_unlink(SELEM(sdata));
43+
bpf_selem_unlink(SELEM(sdata), true);
4444

4545
return 0;
4646
}
@@ -75,8 +75,8 @@ void bpf_sk_storage_free(struct sock *sk)
7575
* sk_storage.
7676
*/
7777
bpf_selem_unlink_map(selem);
78-
free_sk_storage = bpf_selem_unlink_storage_nolock(sk_storage,
79-
selem, true);
78+
free_sk_storage = bpf_selem_unlink_storage_nolock(
79+
sk_storage, selem, true, false);
8080
}
8181
raw_spin_unlock_bh(&sk_storage->lock);
8282
rcu_read_unlock();

0 commit comments

Comments
 (0)