diff mbox series

[bpf-next,v5,31/34] bpf: eliminate rlimit-based memory accounting for bpf local storage maps

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

Commit Message

Roman Gushchin Nov. 12, 2020, 10:15 p.m. UTC
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(-)

Comments

Song Liu Nov. 13, 2020, 6:14 p.m. UTC | #1
> 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

[...]
Roman Gushchin Nov. 13, 2020, 7:33 p.m. UTC | #2
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!
Song Liu Nov. 13, 2020, 8:53 p.m. UTC | #3
> 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 mbox series

Patch

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);
 	}