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>
