Thanks for testing. Sure, I will re-submit with the required changes.
On Thu, Jan 31, 2019, at 9:15 AM, Y Song wrote:
> On Wed, Jan 30, 2019 at 6:05 AM Martynas Pumputis <m...@lambda.lt> wrote:
> >
> > Previously, memory allocated for a map was not accounted. Therefore,
> > this memory could not be taken into consideration by the cgroups
> > memory controller.
> >
> > This patch introduces the "BPF_F_ACCOUNT_MEM" flag which enables
> > the memory accounting for a map, and it can be set during
> > the map creation ("BPF_MAP_CREATE") in "map_flags".
> >
> > When enabled, we account only that amount of memory which is charged
> > against the "RLIMIT_MEMLOCK" limit.
> >
> > To validate the change, first we create the memory cgroup "test-map":
> >
> > # mkdir /sys/fs/cgroup/memory/test-map
> >
> > And then we run the following program against the cgroup:
> >
> > $ cat test_map.c
> > <..>
> > int main() {
> > usleep(3 * 1000000);
> > assert(bpf_create_map(BPF_MAP_TYPE_HASH, 8, 16, 65536, 0) > 0);
> > usleep(3 * 1000000);
> > }
> > # cgexec -g memory:test-map ./test_map &
> > # cat /sys/fs/cgroup/memory/test-map/memory{,.kmem}.usage_in_bytes
> > 397312
> > 258048
> >
> > <after 3 sec the map has been created>
> >
> > # bpftool map list
> > 19: hash flags 0x0
> > key 8B value 16B max_entries 65536 memlock 5771264B
> > # cat /sys/fs/cgroup/memory/test-map/memory{,.kmem}.usage_in_bytes
> > 401408
> > 262144
> >
> > As we can see, the memory allocated for map is not accounted, as
> > 397312B + 5771264B > 401408B.
> >
> > Next, we enabled the accounting and re-run the test:
> >
> > $ cat test_map.c
> > <..>
> > int main() {
> > usleep(3 * 1000000);
> > assert(bpf_create_map(BPF_MAP_TYPE_HASH, 8, 16, 65536,
> > BPF_F_ACCOUNT_MEM) > 0);
> > usleep(3 * 1000000);
> > }
> > # cgexec -g memory:test-map ./test_map &
> > # cat /sys/fs/cgroup/memory/test-map/memory{,.kmem}.usage_in_bytes
> > 450560
> > 307200
> >
> > <after 3 sec the map has been created>
> >
> > # bpftool map list
> > 20: hash flags 0x80
> > key 8B value 16B max_entries 65536 memlock 5771264B
> > # cat /sys/fs/cgroup/memory/test-map/memory{,.kmem}.usage_in_bytes
> > 6221824
> > 6078464
> >
> > This time, the memory (including kmem) is accounted, as
> > 450560B + 5771264B <= 6221824B
>
> The above test result is for cgroup v1.
> I also tested the patch for cgroup v2. It works fine too.
>
> >
> > Signed-off-by: Martynas Pumputis <m...@lambda.lt>
> > ---
> > include/linux/bpf.h | 5 +++--
> > include/uapi/linux/bpf.h | 2 ++
> > kernel/bpf/arraymap.c | 14 +++++++++-----
> > kernel/bpf/bpf_lru_list.c | 11 +++++++++--
> > kernel/bpf/bpf_lru_list.h | 1 +
> > kernel/bpf/cpumap.c | 12 +++++++++---
> > kernel/bpf/devmap.c | 10 ++++++++--
> > kernel/bpf/hashtab.c | 19 ++++++++++++++-----
> > kernel/bpf/lpm_trie.c | 19 +++++++++++++------
> > kernel/bpf/queue_stack_maps.c | 5 +++--
> > kernel/bpf/reuseport_array.c | 3 ++-
> > kernel/bpf/stackmap.c | 12 ++++++++----
> > kernel/bpf/syscall.c | 12 ++++++++----
> > kernel/bpf/xskmap.c | 9 +++++++--
> > net/core/sock_map.c | 13 +++++++++----
> > tools/include/uapi/linux/bpf.h | 3 +++
> > 16 files changed, 108 insertions(+), 42 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index e734f163bd0b..353a3f4304fe 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -79,7 +79,8 @@ struct bpf_map {
> > u32 btf_value_type_id;
> > struct btf *btf;
> > bool unpriv_array;
> > - /* 55 bytes hole */
> > + bool account_mem;
> > + /* 54 bytes hole */
> >
> > /* The 3rd and 4th cacheline with misc members to avoid false
> > sharing
> > * particularly with refcounting.
> > @@ -506,7 +507,7 @@ void bpf_map_put(struct bpf_map *map);
> > int bpf_map_precharge_memlock(u32 pages);
> > int bpf_map_charge_memlock(struct bpf_map *map, u32 pages);
> > void bpf_map_uncharge_memlock(struct bpf_map *map, u32 pages);
> > -void *bpf_map_area_alloc(size_t size, int numa_node);
> > +void *bpf_map_area_alloc(size_t size, int numa_node, bool account_mem);
> > void bpf_map_area_free(void *base);
> > void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr);
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 91c43884f295..a374ccbaa51b 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -278,6 +278,8 @@ enum bpf_attach_type {
> > #define BPF_F_NO_COMMON_LRU (1U << 1)
> > /* Specify numa node during map creation */
> > #define BPF_F_NUMA_NODE (1U << 2)
> > +/* Enable memory accounting for map */
> > +#define BPF_F_ACCOUNT_MEM (1U << 7)
>
> Could you put the above definition after BPF_F_ZERO_SEED (1U << 6)?
> The same for tools.
> With this change, you can add my ack:
> Acked-by: Yonghong Song <y...@fb.com>
