| Message ID | 20200727184506.2279656-23-guro@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: switch to memcg-based memory accounting | expand |
On Mon, Jul 27, 2020 at 12:22 PM Roman Gushchin <guro@fb.com> wrote: > > Do not use rlimit-based memory accounting for bpf ringbuffer. > It has been replaced with the memcg-based memory accounting. > > bpf_ringbuf_alloc() can't return anything except ERR_PTR(-ENOMEM) > and a valid pointer, so to simplify the code make it return NULL > in the first case. This allows to drop a couple of lines in > ringbuf_map_alloc() and also makes it look similar to other memory > allocating function like kmalloc(). > > Signed-off-by: Roman Gushchin <guro@fb.com> Acked-by: Song Liu <songliubraving@fb.com> > --- > kernel/bpf/ringbuf.c | 24 ++++-------------------- > 1 file changed, 4 insertions(+), 20 deletions(-) > > diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c > index e8e2c39cbdc9..e687b798d097 100644 > --- a/kernel/bpf/ringbuf.c > +++ b/kernel/bpf/ringbuf.c > @@ -48,7 +48,6 @@ struct bpf_ringbuf { > > struct bpf_ringbuf_map { > struct bpf_map map; > - struct bpf_map_memory memory; > struct bpf_ringbuf *rb; > }; > > @@ -135,7 +134,7 @@ static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node) > > rb = bpf_ringbuf_area_alloc(data_sz, numa_node); > if (!rb) > - return ERR_PTR(-ENOMEM); > + return NULL; > > spin_lock_init(&rb->spinlock); > init_waitqueue_head(&rb->waitq); > @@ -151,8 +150,6 @@ static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node) > static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr) > { > struct bpf_ringbuf_map *rb_map; > - u64 cost; > - int err; > > if (attr->map_flags & ~RINGBUF_CREATE_FLAG_MASK) > return ERR_PTR(-EINVAL); > @@ -174,26 +171,13 @@ static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr) > > bpf_map_init_from_attr(&rb_map->map, attr); > > - cost = sizeof(struct bpf_ringbuf_map) + > - sizeof(struct bpf_ringbuf) + > - attr->max_entries; > - err = bpf_map_charge_init(&rb_map->map.memory, cost); > - if (err) > - goto err_free_map; > - > rb_map->rb = bpf_ringbuf_alloc(attr->max_entries, rb_map->map.numa_node); > - if (IS_ERR(rb_map->rb)) { > - err = PTR_ERR(rb_map->rb); > - goto err_uncharge; > + if (!rb_map->rb) { > + kfree(rb_map); > + return ERR_PTR(-ENOMEM); > } > > return &rb_map->map; > - > -err_uncharge: > - bpf_map_charge_finish(&rb_map->map.memory); > -err_free_map: > - kfree(rb_map); > - return ERR_PTR(err); > } > > static void bpf_ringbuf_free(struct bpf_ringbuf *rb) > -- > 2.26.2 >
On Mon, Jul 27, 2020 at 12:21 PM Roman Gushchin <guro@fb.com> wrote: > > Do not use rlimit-based memory accounting for bpf ringbuffer. > It has been replaced with the memcg-based memory accounting. > > bpf_ringbuf_alloc() can't return anything except ERR_PTR(-ENOMEM) > and a valid pointer, so to simplify the code make it return NULL > in the first case. This allows to drop a couple of lines in > ringbuf_map_alloc() and also makes it look similar to other memory > allocating function like kmalloc(). > > Signed-off-by: Roman Gushchin <guro@fb.com> > --- LGTM. Acked-by: Andrii Nakryiko <andriin@fb.com> > kernel/bpf/ringbuf.c | 24 ++++-------------------- > 1 file changed, 4 insertions(+), 20 deletions(-) > [...]
diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c index e8e2c39cbdc9..e687b798d097 100644 --- a/kernel/bpf/ringbuf.c +++ b/kernel/bpf/ringbuf.c @@ -48,7 +48,6 @@ struct bpf_ringbuf { struct bpf_ringbuf_map { struct bpf_map map; - struct bpf_map_memory memory; struct bpf_ringbuf *rb; }; @@ -135,7 +134,7 @@ static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node) rb = bpf_ringbuf_area_alloc(data_sz, numa_node); if (!rb) - return ERR_PTR(-ENOMEM); + return NULL; spin_lock_init(&rb->spinlock); init_waitqueue_head(&rb->waitq); @@ -151,8 +150,6 @@ static struct bpf_ringbuf *bpf_ringbuf_alloc(size_t data_sz, int numa_node) static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr) { struct bpf_ringbuf_map *rb_map; - u64 cost; - int err; if (attr->map_flags & ~RINGBUF_CREATE_FLAG_MASK) return ERR_PTR(-EINVAL); @@ -174,26 +171,13 @@ static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr) bpf_map_init_from_attr(&rb_map->map, attr); - cost = sizeof(struct bpf_ringbuf_map) + - sizeof(struct bpf_ringbuf) + - attr->max_entries; - err = bpf_map_charge_init(&rb_map->map.memory, cost); - if (err) - goto err_free_map; - rb_map->rb = bpf_ringbuf_alloc(attr->max_entries, rb_map->map.numa_node); - if (IS_ERR(rb_map->rb)) { - err = PTR_ERR(rb_map->rb); - goto err_uncharge; + if (!rb_map->rb) { + kfree(rb_map); + return ERR_PTR(-ENOMEM); } return &rb_map->map; - -err_uncharge: - bpf_map_charge_finish(&rb_map->map.memory); -err_free_map: - kfree(rb_map); - return ERR_PTR(err); } static void bpf_ringbuf_free(struct bpf_ringbuf *rb)
Do not use rlimit-based memory accounting for bpf ringbuffer. It has been replaced with the memcg-based memory accounting. bpf_ringbuf_alloc() can't return anything except ERR_PTR(-ENOMEM) and a valid pointer, so to simplify the code make it return NULL in the first case. This allows to drop a couple of lines in ringbuf_map_alloc() and also makes it look similar to other memory allocating function like kmalloc(). Signed-off-by: Roman Gushchin <guro@fb.com> --- kernel/bpf/ringbuf.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)