Hi Lianbo,
thanks for working on this.
-----Original Message-----
> Kernel commit <80ee81e0403c> ("bpf: Eliminate rlimit-based memory
> accounting infra for bpf maps") removed the struct bpf_map_memory
> member from struct bpf_map. Without the patch, "bpf -m|-M" options
> will print the following errors:
>
> crash> bpf -m 1
> ID BPF_MAP BPF_MAP_TYPE MAP_FLAGS
> 1 ffff96ba41804400 ARRAY 00000000
> KEY_SIZE: 4 VALUE_SIZE: 8 MAX_ENTRIES: 64 MEMLOCK: (unknown)
> ^^^^^^^
> NAME: "dist" UID: (unknown)
> ^^^^^^^
>
> Signed-off-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
> ---
> bpf.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/bpf.c b/bpf.c
> index cb6b0ed385f9..d45e9ab9311b 100644
> --- a/bpf.c
> +++ b/bpf.c
> @@ -15,6 +15,7 @@
> */
>
> #include "defs.h"
> +#include <stdbool.h>
>
> struct bpf_info {
> ulong status;
> @@ -63,6 +64,66 @@ static int do_old_idr(int, ulong, struct list_pair *);
> #define PROG_VERBOSE (0x40)
> #define MAP_VERBOSE (0x80)
>
> +static bool map_is_per_cpu(ulong type)
I think that int is enough here and stdbool.h can be removed.
(also type is int originally.)
> +{
> + /*
> + * See the definition of bpf_map_type:
> + * include/uapi/linux/bpf.h
> + */
> + #define BPF_MAP_TYPE_PERCPU_HASH (5UL)
> + #define BPF_MAP_TYPE_PERCPU_ARRAY (6UL)
> + #define BPF_MAP_TYPE_LRU_PERCPU_HASH (10UL)
> + #define BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE (21UL)
This #define style in function looks unusual.. please let me change.
> +
> + return type == BPF_MAP_TYPE_PERCPU_HASH ||
> + type == BPF_MAP_TYPE_PERCPU_ARRAY ||
> + type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
> + type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE;
> +}
> +
> +static bool map_is_fd_map(ulong type)
> +{
> + /*
> + * See the definition of bpf_map_type:
> + * include/uapi/linux/bpf.h
> + */
> + #define BPF_MAP_TYPE_PROG_ARRAY (3UL)
> + #define BPF_MAP_TYPE_PERF_EVENT_ARRAY (4UL)
> + #define BPF_MAP_TYPE_CGROUP_ARRAY (8UL)
> + #define BPF_MAP_TYPE_ARRAY_OF_MAPS (12UL)
> + #define BPF_MAP_TYPE_HASH_OF_MAPS (13UL)
Ditto.
> +
> + return type == BPF_MAP_TYPE_PROG_ARRAY ||
> + type == BPF_MAP_TYPE_PERF_EVENT_ARRAY ||
> + type == BPF_MAP_TYPE_CGROUP_ARRAY ||
> + type == BPF_MAP_TYPE_ARRAY_OF_MAPS ||
> + type == BPF_MAP_TYPE_HASH_OF_MAPS;
> +
> +}
> +
> +static ulong bpf_map_memory_size(ulong map_type, ulong vsize, ulong ksize, ulong esize)
The arguments are int and uint, and let's sync with kernel for readability.
static ulong bpf_map_memory_size(int map_type, uint value_size,
uint key_size, uint max_entries)
> +{
> + ulong memsize,valsize;
> + int cpus = 0;
> +
> + valsize = vsize;
> +
> + if (map_is_fd_map(map_type))
> + valsize = sizeof(ulong);
This should be uint.
else if (IS_FD_MAP(map))
return sizeof(u32);
> +
> + if (map_is_per_cpu(map_type)) {
> + cpus = get_cpus_possible();
> + if (!cpus)
> + error(WARNING, "cpu_possible_map does not exist, pissible cpus: %d\n", cpus);
s/pissible/possible/
And if this fails, I think it would be better to print "(unknown)", so
let's return 0 here.
> +
> + valsize = roundup(vsize, 8) * cpus;
> + }
> +
> + memsize = roundup((ksize + valsize), 8);
> +
> + return roundup((esize * memsize), PAGESIZE());
> +}
> +
> void
> cmd_bpf(void)
> {
> @@ -332,7 +393,7 @@ do_bpf(ulong flags, ulong prog_id, ulong map_id, int radix)
> {
> struct bpf_info *bpf;
> int i, c, found, entries, type;
> - uint uid, map_pages, key_size, value_size, max_entries;
> + uint uid, map_pages, key_size = 0, value_size = 0, max_entries = 0;
> ulong bpf_prog_aux, bpf_func, end_func, addr, insnsi, user;
> ulong do_progs, do_maps;
> ulonglong load_time;
> @@ -603,7 +664,7 @@ do_map_only:
> map_pages = UINT(bpf->bpf_map_buf + OFFSET(bpf_map_pages));
> fprintf(fp, "%d\n", map_pages * PAGESIZE());
> } else
> - fprintf(fp, "(unknown)\n");
> + fprintf(fp, "%ld\n", bpf_map_memory_size(type, value_size, key_size,
> max_entries));
Then, how about this?
+ } else if (memory = bpf_map_memory_size(type, value_size, key_size, max_entries))
+ fprintf(fp, "%ld\n", memory);
+ else
+ fprintf(fp, "(unknown)");
I've attached a modified patch, could you check?
Thanks,
Kazu
>
> fprintf(fp, " NAME: ");
> if (VALID_MEMBER(bpf_map_name)) {
> @@ -632,7 +693,7 @@ do_map_only:
> else
> fprintf(fp, "(unknown)\n");
> } else
> - fprintf(fp, "(unknown)\n");
> + fprintf(fp, "(unused)\n");
> }
>
> if (flags & DUMP_STRUCT) {
> --
> 2.20.1
Attachment:
0001-Fix-for-bpf-m-M-options-to-appropriately-display-MEM.patch
Description: 0001-Fix-for-bpf-m-M-options-to-appropriately-display-MEM.patch
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility
