Skip to content

Commit bf1c16d

Browse files
committed
bpf: Remove gfp_flags from local storage helpers
With kmalloc_nolock(), we no longer need to pass gfp_flags from bpf() syscalls and helpers for local storage. Remove the argument and fixup in the verifier. Signed-off-by: Amery Hung <[email protected]>
1 parent ef241da commit bf1c16d

File tree

7 files changed

+35
-75
lines changed

7 files changed

+35
-75
lines changed

include/linux/bpf_local_storage.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,19 +180,18 @@ void bpf_selem_link_map(struct bpf_local_storage_map *smap,
180180

181181
struct bpf_local_storage_elem *
182182
bpf_selem_alloc(struct bpf_local_storage_map *smap, void *owner, void *value,
183-
bool swap_uptrs, gfp_t gfp_flags);
183+
bool swap_uptrs);
184184

185185
void bpf_selem_free(struct bpf_local_storage_elem *selem, bool reuse_now);
186186

187187
int
188188
bpf_local_storage_alloc(void *owner,
189189
struct bpf_local_storage_map *smap,
190-
struct bpf_local_storage_elem *first_selem,
191-
gfp_t gfp_flags);
190+
struct bpf_local_storage_elem *first_selem);
192191

193192
struct bpf_local_storage_data *
194193
bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
195-
void *value, u64 map_flags, bool swap_uptrs, gfp_t gfp_flags);
194+
void *value, u64 map_flags, bool swap_uptrs);
196195

197196
u64 bpf_local_storage_map_mem_usage(const struct bpf_map *map);
198197

kernel/bpf/bpf_cgrp_storage.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ static long bpf_cgrp_storage_update_elem(struct bpf_map *map, void *key,
104104

105105
bpf_cgrp_storage_lock();
106106
sdata = bpf_local_storage_update(cgroup, (struct bpf_local_storage_map *)map,
107-
value, map_flags, false, GFP_ATOMIC);
107+
value, map_flags, false);
108108
bpf_cgrp_storage_unlock();
109109
cgroup_put(cgroup);
110110
return PTR_ERR_OR_ZERO(sdata);
@@ -154,9 +154,8 @@ static void cgroup_storage_map_free(struct bpf_map *map)
154154
bpf_local_storage_map_free(map, &cgroup_cache, &bpf_cgrp_storage_busy);
155155
}
156156

157-
/* *gfp_flags* is a hidden argument provided by the verifier */
158-
BPF_CALL_5(bpf_cgrp_storage_get, struct bpf_map *, map, struct cgroup *, cgroup,
159-
void *, value, u64, flags, gfp_t, gfp_flags)
157+
BPF_CALL_4(bpf_cgrp_storage_get, struct bpf_map *, map, struct cgroup *, cgroup,
158+
void *, value, u64, flags)
160159
{
161160
struct bpf_local_storage_data *sdata;
162161
bool nobusy;
@@ -178,7 +177,7 @@ BPF_CALL_5(bpf_cgrp_storage_get, struct bpf_map *, map, struct cgroup *, cgroup,
178177
if (!percpu_ref_is_dying(&cgroup->self.refcnt) &&
179178
(flags & BPF_LOCAL_STORAGE_GET_F_CREATE) && nobusy)
180179
sdata = bpf_local_storage_update(cgroup, (struct bpf_local_storage_map *)map,
181-
value, BPF_NOEXIST, false, gfp_flags);
180+
value, BPF_NOEXIST, false);
182181

183182
unlock:
184183
if (nobusy)

kernel/bpf/bpf_inode_storage.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ static long bpf_fd_inode_storage_update_elem(struct bpf_map *map, void *key,
9898

9999
sdata = bpf_local_storage_update(file_inode(fd_file(f)),
100100
(struct bpf_local_storage_map *)map,
101-
value, map_flags, false, GFP_ATOMIC);
101+
value, map_flags, false);
102102
return PTR_ERR_OR_ZERO(sdata);
103103
}
104104

@@ -124,9 +124,8 @@ static long bpf_fd_inode_storage_delete_elem(struct bpf_map *map, void *key)
124124
return inode_storage_delete(file_inode(fd_file(f)), map);
125125
}
126126

127-
/* *gfp_flags* is a hidden argument provided by the verifier */
128-
BPF_CALL_5(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode,
129-
void *, value, u64, flags, gfp_t, gfp_flags)
127+
BPF_CALL_4(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode,
128+
void *, value, u64, flags)
130129
{
131130
struct bpf_local_storage_data *sdata;
132131

@@ -152,7 +151,7 @@ BPF_CALL_5(bpf_inode_storage_get, struct bpf_map *, map, struct inode *, inode,
152151
if (flags & BPF_LOCAL_STORAGE_GET_F_CREATE) {
153152
sdata = bpf_local_storage_update(
154153
inode, (struct bpf_local_storage_map *)map, value,
155-
BPF_NOEXIST, false, gfp_flags);
154+
BPF_NOEXIST, false);
156155
return IS_ERR(sdata) ? (unsigned long)NULL :
157156
(unsigned long)sdata->data;
158157
}

kernel/bpf/bpf_local_storage.c

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ static bool selem_linked_to_map(const struct bpf_local_storage_elem *selem)
7373

7474
struct bpf_local_storage_elem *
7575
bpf_selem_alloc(struct bpf_local_storage_map *smap, void *owner,
76-
void *value, bool swap_uptrs, gfp_t gfp_flags)
76+
void *value, bool swap_uptrs)
7777
{
7878
struct bpf_local_storage_elem *selem;
7979

@@ -339,8 +339,7 @@ static int check_flags(const struct bpf_local_storage_data *old_sdata,
339339

340340
int bpf_local_storage_alloc(void *owner,
341341
struct bpf_local_storage_map *smap,
342-
struct bpf_local_storage_elem *first_selem,
343-
gfp_t gfp_flags)
342+
struct bpf_local_storage_elem *first_selem)
344343
{
345344
struct bpf_local_storage *prev_storage, *storage;
346345
struct bpf_local_storage **owner_storage_ptr;
@@ -399,7 +398,7 @@ int bpf_local_storage_alloc(void *owner,
399398
*/
400399
struct bpf_local_storage_data *
401400
bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
402-
void *value, u64 map_flags, bool swap_uptrs, gfp_t gfp_flags)
401+
void *value, u64 map_flags, bool swap_uptrs)
403402
{
404403
struct bpf_local_storage_data *old_sdata = NULL;
405404
struct bpf_local_storage_elem *alloc_selem, *selem = NULL;
@@ -415,9 +414,6 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
415414
!btf_record_has_field(smap->map.record, BPF_SPIN_LOCK)))
416415
return ERR_PTR(-EINVAL);
417416

418-
if (gfp_flags == GFP_KERNEL && (map_flags & ~BPF_F_LOCK) != BPF_NOEXIST)
419-
return ERR_PTR(-EINVAL);
420-
421417
local_storage = rcu_dereference_check(*owner_storage(smap, owner),
422418
bpf_rcu_lock_held());
423419
if (!local_storage || hlist_empty(&local_storage->list)) {
@@ -426,11 +422,11 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
426422
if (err)
427423
return ERR_PTR(err);
428424

429-
selem = bpf_selem_alloc(smap, owner, value, swap_uptrs, gfp_flags);
425+
selem = bpf_selem_alloc(smap, owner, value, swap_uptrs);
430426
if (!selem)
431427
return ERR_PTR(-ENOMEM);
432428

433-
err = bpf_local_storage_alloc(owner, smap, selem, gfp_flags);
429+
err = bpf_local_storage_alloc(owner, smap, selem);
434430
if (err) {
435431
bpf_selem_free(selem, true);
436432
mem_uncharge(smap, owner, smap->elem_size);
@@ -460,7 +456,7 @@ bpf_local_storage_update(void *owner, struct bpf_local_storage_map *smap,
460456
/* A lookup has just been done before and concluded a new selem is
461457
* needed. The chance of an unnecessary alloc is unlikely.
462458
*/
463-
alloc_selem = selem = bpf_selem_alloc(smap, owner, value, swap_uptrs, gfp_flags);
459+
alloc_selem = selem = bpf_selem_alloc(smap, owner, value, swap_uptrs);
464460
if (!alloc_selem)
465461
return ERR_PTR(-ENOMEM);
466462

kernel/bpf/bpf_task_storage.c

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ static long bpf_pid_task_storage_update_elem(struct bpf_map *map, void *key,
146146
bpf_task_storage_lock();
147147
sdata = bpf_local_storage_update(
148148
task, (struct bpf_local_storage_map *)map, value, map_flags,
149-
true, GFP_ATOMIC);
149+
true);
150150
bpf_task_storage_unlock();
151151

152152
err = PTR_ERR_OR_ZERO(sdata);
@@ -205,7 +205,7 @@ static long bpf_pid_task_storage_delete_elem(struct bpf_map *map, void *key)
205205
/* Called by bpf_task_storage_get*() helpers */
206206
static void *__bpf_task_storage_get(struct bpf_map *map,
207207
struct task_struct *task, void *value,
208-
u64 flags, gfp_t gfp_flags, bool nobusy)
208+
u64 flags, bool nobusy)
209209
{
210210
struct bpf_local_storage_data *sdata;
211211

@@ -218,16 +218,15 @@ static void *__bpf_task_storage_get(struct bpf_map *map,
218218
(flags & BPF_LOCAL_STORAGE_GET_F_CREATE) && nobusy) {
219219
sdata = bpf_local_storage_update(
220220
task, (struct bpf_local_storage_map *)map, value,
221-
BPF_NOEXIST, false, gfp_flags);
221+
BPF_NOEXIST, false);
222222
return IS_ERR(sdata) ? NULL : sdata->data;
223223
}
224224

225225
return NULL;
226226
}
227227

228-
/* *gfp_flags* is a hidden argument provided by the verifier */
229-
BPF_CALL_5(bpf_task_storage_get_recur, struct bpf_map *, map, struct task_struct *,
230-
task, void *, value, u64, flags, gfp_t, gfp_flags)
228+
BPF_CALL_4(bpf_task_storage_get_recur, struct bpf_map *, map, struct task_struct *,
229+
task, void *, value, u64, flags)
231230
{
232231
bool nobusy;
233232
void *data;
@@ -237,16 +236,14 @@ BPF_CALL_5(bpf_task_storage_get_recur, struct bpf_map *, map, struct task_struct
237236
return (unsigned long)NULL;
238237

239238
nobusy = bpf_task_storage_trylock();
240-
data = __bpf_task_storage_get(map, task, value, flags,
241-
gfp_flags, nobusy);
239+
data = __bpf_task_storage_get(map, task, value, flags, nobusy);
242240
if (nobusy)
243241
bpf_task_storage_unlock();
244242
return (unsigned long)data;
245243
}
246244

247-
/* *gfp_flags* is a hidden argument provided by the verifier */
248-
BPF_CALL_5(bpf_task_storage_get, struct bpf_map *, map, struct task_struct *,
249-
task, void *, value, u64, flags, gfp_t, gfp_flags)
245+
BPF_CALL_4(bpf_task_storage_get, struct bpf_map *, map, struct task_struct *,
246+
task, void *, value, u64, flags)
250247
{
251248
void *data;
252249

@@ -255,8 +252,7 @@ BPF_CALL_5(bpf_task_storage_get, struct bpf_map *, map, struct task_struct *,
255252
return (unsigned long)NULL;
256253

257254
bpf_task_storage_lock();
258-
data = __bpf_task_storage_get(map, task, value, flags,
259-
gfp_flags, true);
255+
data = __bpf_task_storage_get(map, task, value, flags, true);
260256
bpf_task_storage_unlock();
261257
return (unsigned long)data;
262258
}

kernel/bpf/verifier.c

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -571,14 +571,6 @@ static bool is_may_goto_insn_at(struct bpf_verifier_env *env, int insn_idx)
571571
return is_may_goto_insn(&env->prog->insnsi[insn_idx]);
572572
}
573573

574-
static bool is_storage_get_function(enum bpf_func_id func_id)
575-
{
576-
return func_id == BPF_FUNC_sk_storage_get ||
577-
func_id == BPF_FUNC_inode_storage_get ||
578-
func_id == BPF_FUNC_task_storage_get ||
579-
func_id == BPF_FUNC_cgrp_storage_get;
580-
}
581-
582574
static bool helper_multiple_ref_obj_use(enum bpf_func_id func_id,
583575
const struct bpf_map *map)
584576
{
@@ -23007,24 +22999,6 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
2300722999
goto patch_call_imm;
2300823000
}
2300923001

23010-
if (is_storage_get_function(insn->imm)) {
23011-
if (env->insn_aux_data[i + delta].non_sleepable)
23012-
insn_buf[0] = BPF_MOV64_IMM(BPF_REG_5, (__force __s32)GFP_ATOMIC);
23013-
else
23014-
insn_buf[0] = BPF_MOV64_IMM(BPF_REG_5, (__force __s32)GFP_KERNEL);
23015-
insn_buf[1] = *insn;
23016-
cnt = 2;
23017-
23018-
new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
23019-
if (!new_prog)
23020-
return -ENOMEM;
23021-
23022-
delta += cnt - 1;
23023-
env->prog = prog = new_prog;
23024-
insn = new_prog->insnsi + i + delta;
23025-
goto patch_call_imm;
23026-
}
23027-
2302823002
/* bpf_per_cpu_ptr() and bpf_this_cpu_ptr() */
2302923003
if (env->insn_aux_data[i + delta].call_with_percpu_alloc_ptr) {
2303023004
/* patch with 'r1 = *(u64 *)(r1 + 0)' since for percpu data,

net/core/bpf_sk_storage.c

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ static long bpf_fd_sk_storage_update_elem(struct bpf_map *map, void *key,
105105
if (sock) {
106106
sdata = bpf_local_storage_update(
107107
sock->sk, (struct bpf_local_storage_map *)map, value,
108-
map_flags, false, GFP_ATOMIC);
108+
map_flags, false);
109109
sockfd_put(sock);
110110
return PTR_ERR_OR_ZERO(sdata);
111111
}
@@ -136,7 +136,7 @@ bpf_sk_storage_clone_elem(struct sock *newsk,
136136
{
137137
struct bpf_local_storage_elem *copy_selem;
138138

139-
copy_selem = bpf_selem_alloc(smap, newsk, NULL, false, GFP_ATOMIC);
139+
copy_selem = bpf_selem_alloc(smap, newsk, NULL, false);
140140
if (!copy_selem)
141141
return NULL;
142142

@@ -194,7 +194,7 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
194194
bpf_selem_link_map(smap, copy_selem);
195195
bpf_selem_link_storage_nolock(new_sk_storage, copy_selem);
196196
} else {
197-
ret = bpf_local_storage_alloc(newsk, smap, copy_selem, GFP_ATOMIC);
197+
ret = bpf_local_storage_alloc(newsk, smap, copy_selem);
198198
if (ret) {
199199
bpf_selem_free(copy_selem, true);
200200
atomic_sub(smap->elem_size,
@@ -219,9 +219,8 @@ int bpf_sk_storage_clone(const struct sock *sk, struct sock *newsk)
219219
return ret;
220220
}
221221

222-
/* *gfp_flags* is a hidden argument provided by the verifier */
223-
BPF_CALL_5(bpf_sk_storage_get, struct bpf_map *, map, struct sock *, sk,
224-
void *, value, u64, flags, gfp_t, gfp_flags)
222+
BPF_CALL_4(bpf_sk_storage_get, struct bpf_map *, map, struct sock *, sk,
223+
void *, value, u64, flags)
225224
{
226225
struct bpf_local_storage_data *sdata;
227226

@@ -242,7 +241,7 @@ BPF_CALL_5(bpf_sk_storage_get, struct bpf_map *, map, struct sock *, sk,
242241
refcount_inc_not_zero(&sk->sk_refcnt)) {
243242
sdata = bpf_local_storage_update(
244243
sk, (struct bpf_local_storage_map *)map, value,
245-
BPF_NOEXIST, false, gfp_flags);
244+
BPF_NOEXIST, false);
246245
/* sk must be a fullsock (guaranteed by verifier),
247246
* so sock_gen_put() is unnecessary.
248247
*/
@@ -374,16 +373,14 @@ static bool bpf_sk_storage_tracing_allowed(const struct bpf_prog *prog)
374373
return false;
375374
}
376375

377-
/* *gfp_flags* is a hidden argument provided by the verifier */
378-
BPF_CALL_5(bpf_sk_storage_get_tracing, struct bpf_map *, map, struct sock *, sk,
379-
void *, value, u64, flags, gfp_t, gfp_flags)
376+
BPF_CALL_4(bpf_sk_storage_get_tracing, struct bpf_map *, map, struct sock *, sk,
377+
void *, value, u64, flags)
380378
{
381379
WARN_ON_ONCE(!bpf_rcu_lock_held());
382380
if (in_hardirq() || in_nmi())
383381
return (unsigned long)NULL;
384382

385-
return (unsigned long)____bpf_sk_storage_get(map, sk, value, flags,
386-
gfp_flags);
383+
return (unsigned long)____bpf_sk_storage_get(map, sk, value, flags);
387384
}
388385

389386
BPF_CALL_2(bpf_sk_storage_delete_tracing, struct bpf_map *, map,

0 commit comments

Comments
 (0)