On Mon, Sep 28, 2020 at 7:14 AM Alan Maguire alan.maguire@oracle.com wrote:
On Thu, 24 Sep 2020, Alexei Starovoitov wrote:
to whatever number, but printing single task_struct needs ~800 lines and ~18kbytes. Humans can scroll through that much spam, but can we make it less verbose by default somehow? May be not in this patch set, but in the follow up?
One approach that might work would be to devote 4 bits or so of flag space to a "maximum depth" specifier; i.e. at depth 1, only base types are displayed, no aggregate types like arrays, structs and unions. We've already got depth processing in the code to figure out if possibly zeroed nested data needs to be displayed, so it should hopefully be a simple follow-up.
One way to express it would be to use "..." to denote field(s) were omitted. We could even use the number of "."s to denote cases where multiple fields were omitted, giving a visual sense of how much data was omitted. So for example with BTF_F_MAX_DEPTH(1), task_struct looks like this:
(struct task_struct){ .state = ()1, .stack = ( *)0x00000000029d1e6f, ... .flags = (unsigned int)4194560, ... .cpu = (unsigned int)36, .wakee_flips = (unsigned int)11, .wakee_flip_decay_ts = (long unsigned int)4294914874, .last_wakee = (struct task_struct *)0x000000006c7dfe6d, .recent_used_cpu = (int)19, .wake_cpu = (int)36, .prio = (int)120, .static_prio = (int)120, .normal_prio = (int)120, .sched_class = (struct sched_class *)0x00000000ad1561e6, ... .exec_start = (u64)674402577156, .sum_exec_runtime = (u64)5009664110, .vruntime = (u64)167038057, .prev_sum_exec_runtime = (u64)5009578167, .nr_migrations = (u64)54, .depth = (int)1, .parent = (struct sched_entity *)0x00000000cba60e7d, .cfs_rq = (struct cfs_rq *)0x0000000014f353ed, ...
...etc. What do you think?
It's not clear to me what exactly is omitted with ... ? Would it make sense to still at least list a field name and "abbreviated" value. E.g., for arrays:
.array_field = (int[16]){ ... },
Similarly for struct:
.struct_field = (struct my_struct){ ... },
? With just '...' I get a very strong and unsettling feeling of missing out on the important stuff :)
+SEC("iter/task") +int dump_task_fs_struct(struct bpf_iter__task *ctx) +{
- static const char fs_type[] = "struct fs_struct";
- struct seq_file *seq = ctx->meta->seq;
- struct task_struct *task = ctx->task;
- struct fs_struct *fs = (void *)0;
- static struct btf_ptr ptr = { };
- long ret;
- if (task)
fs = task->fs;
- ptr.type = fs_type;
- ptr.ptr = fs;
imo the following is better: ptr.type_id = __builtin_btf_type_id(*fs, 1); ptr.ptr = fs;
I'm still seeing lookup failures using __builtin_btf_type_id(,1) - whereas both __builtin_btf_type_id(,0) and Andrii's suggestion of bpf_core_type_id_kernel() work. Not sure what's going on - pahole is v1.17, clang is
bpf_core_type_id_kernel() is
__builtin_btf_type_id(*(typeof(type) *)0, BPF_TYPE_ID_TARGET)
BPF_TYPE_ID_TARGET is exactly 1. So I bet it's because of the type capturing through typeof() and pointer casting/dereferencing, which preserves type information properly. Regardless, just use the helper, IMO.
clang version 12.0.0 (/mnt/src/llvm-project/clang 7ab7b979d29e1e43701cf690f5cf1903740f50e3)
[...]