| Message ID | 20201112221543.3621014-32-guro@fb.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: switch to memcg-based memory accounting | expand |
> On Nov 12, 2020, at 2:15 PM, Roman Gushchin <guro@fb.com> wrote: > > Do not use rlimit-based memory accounting for bpf local storage maps. > It has been replaced with the memcg-based memory accounting. > > Signed-off-by: Roman Gushchin <guro@fb.com> > --- > kernel/bpf/bpf_local_storage.c | 11 ----------- > 1 file changed, 11 deletions(-) > > diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c > index fd4f9ac1d042..3b0da5a04d55 100644 > --- a/kernel/bpf/bpf_local_storage.c > +++ b/kernel/bpf/bpf_local_storage.c Do we need to change/remove mem_charge() and mem_uncharge() in bpf_local_storage.c? I didn't find that in the set. Thanks, Song [...]
On Fri, Nov 13, 2020 at 10:14:48AM -0800, Song Liu wrote: > > > > On Nov 12, 2020, at 2:15 PM, Roman Gushchin <guro@fb.com> wrote: > > > > Do not use rlimit-based memory accounting for bpf local storage maps. > > It has been replaced with the memcg-based memory accounting. > > > > Signed-off-by: Roman Gushchin <guro@fb.com> > > --- > > kernel/bpf/bpf_local_storage.c | 11 ----------- > > 1 file changed, 11 deletions(-) > > > > diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c > > index fd4f9ac1d042..3b0da5a04d55 100644 > > --- a/kernel/bpf/bpf_local_storage.c > > +++ b/kernel/bpf/bpf_local_storage.c > > Do we need to change/remove mem_charge() and mem_uncharge() in > bpf_local_storage.c? I didn't find that in the set. No, those are used for per-socket memory limits (see sk_storage_charge() and omem_charge()). Btw, thanks for looking into the patchset!
> On Nov 13, 2020, at 11:33 AM, Roman Gushchin <guro@fb.com> wrote: > > On Fri, Nov 13, 2020 at 10:14:48AM -0800, Song Liu wrote: >> >> >>> On Nov 12, 2020, at 2:15 PM, Roman Gushchin <guro@fb.com> wrote: >>> >>> Do not use rlimit-based memory accounting for bpf local storage maps. >>> It has been replaced with the memcg-based memory accounting. >>> >>> Signed-off-by: Roman Gushchin <guro@fb.com> >>> --- >>> kernel/bpf/bpf_local_storage.c | 11 ----------- >>> 1 file changed, 11 deletions(-) >>> >>> diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c >>> index fd4f9ac1d042..3b0da5a04d55 100644 >>> --- a/kernel/bpf/bpf_local_storage.c >>> +++ b/kernel/bpf/bpf_local_storage.c >> >> Do we need to change/remove mem_charge() and mem_uncharge() in >> bpf_local_storage.c? I didn't find that in the set. > > No, those are used for per-socket memory limits (see sk_storage_charge() > and omem_charge()). I see. Thanks for the explanation. Acked-by: Song Liu <songliubraving@fb.com>
diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c index fd4f9ac1d042..3b0da5a04d55 100644 --- a/kernel/bpf/bpf_local_storage.c +++ b/kernel/bpf/bpf_local_storage.c @@ -544,8 +544,6 @@ struct bpf_local_storage_map *bpf_local_storage_map_alloc(union bpf_attr *attr) struct bpf_local_storage_map *smap; unsigned int i; u32 nbuckets; - u64 cost; - int ret; smap = kzalloc(sizeof(*smap), GFP_USER | __GFP_NOWARN | __GFP_ACCOUNT); if (!smap) @@ -556,18 +554,9 @@ struct bpf_local_storage_map *bpf_local_storage_map_alloc(union bpf_attr *attr) /* Use at least 2 buckets, select_bucket() is undefined behavior with 1 bucket */ nbuckets = max_t(u32, 2, nbuckets); smap->bucket_log = ilog2(nbuckets); - cost = sizeof(*smap->buckets) * nbuckets + sizeof(*smap); - - ret = bpf_map_charge_init(&smap->map.memory, cost); - if (ret < 0) { - kfree(smap); - return ERR_PTR(ret); - } - smap->buckets = kvcalloc(sizeof(*smap->buckets), nbuckets, GFP_USER | __GFP_NOWARN | __GFP_ACCOUNT); if (!smap->buckets) { - bpf_map_charge_finish(&smap->map.memory); kfree(smap); return ERR_PTR(-ENOMEM); }
Do not use rlimit-based memory accounting for bpf local storage maps. It has been replaced with the memcg-based memory accounting. Signed-off-by: Roman Gushchin <guro@fb.com> --- kernel/bpf/bpf_local_storage.c | 11 ----------- 1 file changed, 11 deletions(-)