| Message ID | 20220729152316.58205-6-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bpf: Introduce selectable memcg for bpf map | expand |
On Fri, Jul 29, 2022 at 03:23:06PM +0000, Yafang Shao wrote: > Currently bpf_map_area_alloc() is used to allocate a container of struct > bpf_map or members in this container. To distinguish the map creation > and other members, let split it into two different helpers, > - bpf_map_container_alloc() > Used to allocate a container of struct bpf_map, the container is as > follows, > struct bpf_map_container { > struct bpf_map map; // the map must be the first member > .... > }; > Pls. note that the struct bpf_map_contianer is a abstract one, which > can be struct bpf_array, struct bpf_bloom_filter and etc. > > In this helper, it will call bpf_map_save_memcg() to init memcg > relevant data in the bpf map. And these data will be cleared in > bpf_map_container_free(). > > - bpf_map_area_alloc() > Now it is used to allocate the members in a contianer only. > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > --- > include/linux/bpf.h | 4 ++++ > kernel/bpf/syscall.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 60 insertions(+) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 20c26aed7896..2d971b0eb24b 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1634,9 +1634,13 @@ void bpf_map_inc_with_uref(struct bpf_map *map); > struct bpf_map * __must_check bpf_map_inc_not_zero(struct bpf_map *map); > void bpf_map_put_with_uref(struct bpf_map *map); > void bpf_map_put(struct bpf_map *map); > +void *bpf_map_container_alloc(u64 size, int numa_node); > +void *bpf_map_container_mmapable_alloc(u64 size, int numa_node, > + u32 align, u32 offset); > void *bpf_map_area_alloc(u64 size, int numa_node); > void *bpf_map_area_mmapable_alloc(u64 size, int numa_node); > void bpf_map_area_free(void *base); > +void bpf_map_container_free(void *base); > bool bpf_map_write_active(const struct bpf_map *map); > void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr); > int generic_map_lookup_batch(struct bpf_map *map, > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index 83c7136c5788..1a1a81a11b37 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -495,6 +495,62 @@ static void bpf_map_release_memcg(struct bpf_map *map) > } > #endif > > +/* > + * The return pointer is a bpf_map container, as follow, > + * struct bpf_map_container { > + * struct bpf_map map; > + * ... > + * }; > + * > + * It is used in map creation path. > + */ > +void *bpf_map_container_alloc(u64 size, int numa_node) > +{ > + struct bpf_map *map; > + void *container; > + > + container = __bpf_map_area_alloc(size, numa_node, false); > + if (!container) > + return NULL; > + > + map = (struct bpf_map *)container; > + bpf_map_save_memcg(map); > + > + return container; > +} > + > +void *bpf_map_container_mmapable_alloc(u64 size, int numa_node, u32 align, > + u32 offset) > +{ > + struct bpf_map *map; > + void *container; > + void *ptr; > + > + /* kmalloc'ed memory can't be mmap'ed, use explicit vmalloc */ > + ptr = __bpf_map_area_alloc(size, numa_node, true); > + if (!ptr) > + return NULL; > + > + container = ptr + align - offset; > + map = (struct bpf_map *)container; > + bpf_map_save_memcg(map); This is very error prone. I don't think the container concept is necessary. bpf_map_area_alloc() can just take extra memcg_fd argument. > + > + return ptr; > +} > + > +void bpf_map_container_free(void *container) > +{ > + struct bpf_map *map; > + > + if (!container) > + return; > + > + map = (struct bpf_map *)container; > + bpf_map_release_memcg(map); > + > + kvfree(container); > +} > + > static int bpf_map_kptr_off_cmp(const void *a, const void *b) > { > const struct bpf_map_value_off_desc *off_desc1 = a, *off_desc2 = b; > -- > 2.17.1 >
On Tue, Aug 2, 2022 at 12:58 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Jul 29, 2022 at 03:23:06PM +0000, Yafang Shao wrote: > > Currently bpf_map_area_alloc() is used to allocate a container of struct > > bpf_map or members in this container. To distinguish the map creation > > and other members, let split it into two different helpers, > > - bpf_map_container_alloc() > > Used to allocate a container of struct bpf_map, the container is as > > follows, > > struct bpf_map_container { > > struct bpf_map map; // the map must be the first member > > .... > > }; > > Pls. note that the struct bpf_map_contianer is a abstract one, which > > can be struct bpf_array, struct bpf_bloom_filter and etc. > > > > In this helper, it will call bpf_map_save_memcg() to init memcg > > relevant data in the bpf map. And these data will be cleared in > > bpf_map_container_free(). > > > > - bpf_map_area_alloc() > > Now it is used to allocate the members in a contianer only. > > > > Signed-off-by: Yafang Shao <laoar.shao@gmail.com> > > --- > > include/linux/bpf.h | 4 ++++ > > kernel/bpf/syscall.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 60 insertions(+) > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > index 20c26aed7896..2d971b0eb24b 100644 > > --- a/include/linux/bpf.h > > +++ b/include/linux/bpf.h > > @@ -1634,9 +1634,13 @@ void bpf_map_inc_with_uref(struct bpf_map *map); > > struct bpf_map * __must_check bpf_map_inc_not_zero(struct bpf_map *map); > > void bpf_map_put_with_uref(struct bpf_map *map); > > void bpf_map_put(struct bpf_map *map); > > +void *bpf_map_container_alloc(u64 size, int numa_node); > > +void *bpf_map_container_mmapable_alloc(u64 size, int numa_node, > > + u32 align, u32 offset); > > void *bpf_map_area_alloc(u64 size, int numa_node); > > void *bpf_map_area_mmapable_alloc(u64 size, int numa_node); > > void bpf_map_area_free(void *base); > > +void bpf_map_container_free(void *base); > > bool bpf_map_write_active(const struct bpf_map *map); > > void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr); > > int generic_map_lookup_batch(struct bpf_map *map, > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index 83c7136c5788..1a1a81a11b37 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -495,6 +495,62 @@ static void bpf_map_release_memcg(struct bpf_map *map) > > } > > #endif > > > > +/* > > + * The return pointer is a bpf_map container, as follow, > > + * struct bpf_map_container { > > + * struct bpf_map map; > > + * ... > > + * }; > > + * > > + * It is used in map creation path. > > + */ > > +void *bpf_map_container_alloc(u64 size, int numa_node) > > +{ > > + struct bpf_map *map; > > + void *container; > > + > > + container = __bpf_map_area_alloc(size, numa_node, false); > > + if (!container) > > + return NULL; > > + > > + map = (struct bpf_map *)container; > > + bpf_map_save_memcg(map); > > + > > + return container; > > +} > > + > > +void *bpf_map_container_mmapable_alloc(u64 size, int numa_node, u32 align, > > + u32 offset) > > +{ > > + struct bpf_map *map; > > + void *container; > > + void *ptr; > > + > > + /* kmalloc'ed memory can't be mmap'ed, use explicit vmalloc */ > > + ptr = __bpf_map_area_alloc(size, numa_node, true); > > + if (!ptr) > > + return NULL; > > + > > + container = ptr + align - offset; > > + map = (struct bpf_map *)container; > > + bpf_map_save_memcg(map); > > This is very error prone. > I don't think the container concept is necessary. > bpf_map_area_alloc() can just take extra memcg_fd argument. > Got it. I will change it.
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 20c26aed7896..2d971b0eb24b 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1634,9 +1634,13 @@ void bpf_map_inc_with_uref(struct bpf_map *map); struct bpf_map * __must_check bpf_map_inc_not_zero(struct bpf_map *map); void bpf_map_put_with_uref(struct bpf_map *map); void bpf_map_put(struct bpf_map *map); +void *bpf_map_container_alloc(u64 size, int numa_node); +void *bpf_map_container_mmapable_alloc(u64 size, int numa_node, + u32 align, u32 offset); void *bpf_map_area_alloc(u64 size, int numa_node); void *bpf_map_area_mmapable_alloc(u64 size, int numa_node); void bpf_map_area_free(void *base); +void bpf_map_container_free(void *base); bool bpf_map_write_active(const struct bpf_map *map); void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr); int generic_map_lookup_batch(struct bpf_map *map, diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 83c7136c5788..1a1a81a11b37 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -495,6 +495,62 @@ static void bpf_map_release_memcg(struct bpf_map *map) } #endif +/* + * The return pointer is a bpf_map container, as follow, + * struct bpf_map_container { + * struct bpf_map map; + * ... + * }; + * + * It is used in map creation path. + */ +void *bpf_map_container_alloc(u64 size, int numa_node) +{ + struct bpf_map *map; + void *container; + + container = __bpf_map_area_alloc(size, numa_node, false); + if (!container) + return NULL; + + map = (struct bpf_map *)container; + bpf_map_save_memcg(map); + + return container; +} + +void *bpf_map_container_mmapable_alloc(u64 size, int numa_node, u32 align, + u32 offset) +{ + struct bpf_map *map; + void *container; + void *ptr; + + /* kmalloc'ed memory can't be mmap'ed, use explicit vmalloc */ + ptr = __bpf_map_area_alloc(size, numa_node, true); + if (!ptr) + return NULL; + + container = ptr + align - offset; + map = (struct bpf_map *)container; + bpf_map_save_memcg(map); + + return ptr; +} + +void bpf_map_container_free(void *container) +{ + struct bpf_map *map; + + if (!container) + return; + + map = (struct bpf_map *)container; + bpf_map_release_memcg(map); + + kvfree(container); +} + static int bpf_map_kptr_off_cmp(const void *a, const void *b) { const struct bpf_map_value_off_desc *off_desc1 = a, *off_desc2 = b;
Currently bpf_map_area_alloc() is used to allocate a container of struct bpf_map or members in this container. To distinguish the map creation and other members, let split it into two different helpers, - bpf_map_container_alloc() Used to allocate a container of struct bpf_map, the container is as follows, struct bpf_map_container { struct bpf_map map; // the map must be the first member .... }; Pls. note that the struct bpf_map_contianer is a abstract one, which can be struct bpf_array, struct bpf_bloom_filter and etc. In this helper, it will call bpf_map_save_memcg() to init memcg relevant data in the bpf map. And these data will be cleared in bpf_map_container_free(). - bpf_map_area_alloc() Now it is used to allocate the members in a contianer only. Signed-off-by: Yafang Shao <laoar.shao@gmail.com> --- include/linux/bpf.h | 4 ++++ kernel/bpf/syscall.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+)