On Mon, Sep 28, 2020 at 7:14 AM Alan Maguire <alan.maguire@xxxxxxxxxx> 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)
>
[...]
