Re: [Crash-utility] [PATCH v2 1/1] tools: list: create O option for specifying head node offset

Hi Firo, thanks for updating the patch. -----Original Message----- > This new O option is very useful to specify the head node > offset for listing linked list whose head node embedded has a > different offset to other node, e.g. dentry.d_subdirs(the head node) > and dentry.d_child. > > Signed-off-by: Firo Yang <firo.yang(a)suse.com&gt; > --- > defs.h | 1 + > help.c | 15 ++++++++++++++- > tools.c | 32 +++++++++++++++++++++++++++++--- > 3 files changed, 44 insertions(+), 4 deletions(-) > > diff --git a/defs.h b/defs.h > index 396d61a..d5fcd37 100644 > --- a/defs.h > +++ b/defs.h > @@ -2613,6 +2613,7 @@ struct list_data { /* generic structure used by do_list() to walk */ > #define LIST_PARSE_MEMBER (VERBOSE << 13) > #define LIST_READ_MEMBER (VERBOSE << 14) > #define LIST_BRENT_ALGO (VERBOSE << 15) > +#define LIST_HEAD_OFFSET_ENTERED (VERBOSE << 16) > > struct tree_data { > ulong flags; > diff --git a/help.c b/help.c > index e0c8408..3aa99f0 100644 > --- a/help.c > +++ b/help.c > @@ -5716,7 +5716,7 @@ char *help__list[] = { > "list", > "linked list", > "[[-o] offset][-e end][-[s|S] struct[.member[,member] [-l offset]] -[x|d]]" > -"\n [-r|-B] [-h|-H] start", > +"\n [-r|-B] [-h [-O head_offset]|-H] start", > " ", > " This command dumps the contents of a linked list. The entries in a linked", > " list are typically data structures that are tied together in one of two", > @@ -5800,6 +5800,14 @@ char *help__list[] = { > " -S struct Similar to -s, but instead of parsing gdb output, member values", > " are read directly from memory, so the command works much faster", > " for 1-, 2-, 4-, and 8-byte members.", > +" -O offset The -O option works only with -h option.", > +" It is used for specifying the offset of head node embedded in a", > +" structure, like dentry.d_subdirs or cgroup_subsys_state.children.", I would like to describe it a bit more and generally here, something like: --- Only used in conjunction with -h, specify the offset of a head node list_head that is embedded within a data structure that is different than the offset of list_head within the other structures in the linked list. --- This might be poor, please help to edit..

> +" The offset may be entered in either of two manners:", > +"", > +" 1. \"structure.member\" format.", > +" 2. a number of bytes.", > +"", > " -l offset Only used in conjunction with -s, if the start address argument", > " is a pointer to an embedded list head (or any other similar list", > " linkage structure whose first member points to the next linkage", > @@ -6116,6 +6124,11 @@ char *help__list[] = { > " comm = \"sudo\"", > " ffff88005ac10180", > " comm = \"crash\"", > +"", > +" With the support of '-O' option, we can easily achieve the following convenient", > +" usage: ", > +" %s> alias ls list -O dentry.d_subdirs -o dentry.d_child -s dentry.d_name.name -h ", > +" %s> ls <address of dentry>", That alias is good but it would be better to put an actual output and describe the option more to understand cases that can be used. How about this? --- To display a list that is embedded within a data structure that is different than the offset of list_head within the other structures in the linked list, e.g. dentry.d_subdirs and dentry.d_child, the -O option can be used: %s> list -o dentry.d_child -s dentry.d_name.name -O dentry.d_subdirs -h ffff9c585b81a180 ffff9c585b9cb140 d_name.name = 0xffff9c585b9cb178 "ccc.txt" ffff9c585b9cb980 d_name.name = 0xffff9c585b9cb9b8 "bbb.txt" ffff9c585b9cb740 d_name.name = 0xffff9c585b9cb778 "aaa.txt" The dentry.d_subdirs example above is equal to the following sequence: %s> struct -o dentry.d_subdirs ffff9c585b81a180 struct dentry { [ffff9c585b81a220] struct list_head d_subdirs; } %s> list -o dentry.d_child -s dentry.d_name.name -H ffff9c585b81a220 ---

(just fyi, if you want to list directory entries and inodes, the cacheutils extension module may help you: https://github.com/k-hagio/crash-cacheutils)

> NULL > }; > > diff --git a/tools.c b/tools.c > index a26b101..6a11670 100644 > --- a/tools.c > +++ b/tools.c > @@ -3343,6 +3343,7 @@ void > cmd_list(void) > { > int c; > + long head_member_offset = 0; /* offset for head like denty.d_subdirs */ > struct list_data list_data, *ld; > struct datatype_member struct_member, *sm; > struct syment *sp; > @@ -3353,7 +3354,7 @@ cmd_list(void) > BZERO(ld, sizeof(struct list_data)); > struct_list_offset = 0; > > - while ((c = getopt(argcnt, args, "BHhrs:S:e:o:xdl:")) != EOF) { > + while ((c = getopt(argcnt, args, "BHhrs:S:e:o:O:xdl:")) != EOF) { > switch(c) > { > case 'B': > @@ -3394,6 +3395,24 @@ cmd_list(void) > optarg); > break; > > + case 'O': > + if (ld->flags & LIST_HEAD_OFFSET_ENTERED) > + error(FATAL, > + "offset value %d (0x%lx) already entered\n", > + head_member_offset, head_member_offset); > + else if (IS_A_NUMBER(optarg)) > + head_member_offset = stol(optarg, > + FAULT_ON_ERROR, NULL); > + else if (arg_to_datatype(optarg, > + sm, RETURN_ON_ERROR) > 1) > + head_member_offset = sm->member_offset; > + else > + error(FATAL, "invalid -O argument: %s\n", > + optarg); > + > + ld->flags |= LIST_HEAD_OFFSET_ENTERED; > + break; > + > case 'o': > if (ld->flags & LIST_OFFSET_ENTERED) > error(FATAL, > @@ -3599,8 +3618,15 @@ next_arg: > fprintf(fp, "(empty)\n"); > return; > } > - } else > - ld->start += ld->list_head_offset; > + } else { > + if (ld->flags & LIST_HEAD_OFFSET_ENTERED) { > + if (!ld->end) > + ld->end = ld->start + head_member_offset; > + readmem(ld->start + head_member_offset, KVADDR, > + &ld->start, sizeof(void *), "LIST_HEAD contents", FAULT_ON_ERROR); As wrote above, I understand that the following sequence crash> struct -o dentry.d_subdirs ffff9c585b81a180 crash> list -o dentry.d_child -s dentry.d_name.name -H ffff9c585b81a220 is equal to crash> list -o dentry.d_child -s dentry.d_name.name -O dentry.d_subdirs -h ffff9c585b81a180 with the patch. But if the dentry.d_subdirs list is empty, the two results are not equal: crash> list -o dentry.d_child -s dentry.d_name.name -H ffff9c594fb57de0 (empty) crash> list -o dentry.d_child -s dentry.d_name.name -O dentry.d_subdirs -h ffff9c594fb57d40 ffff9c594fb57d50 d_name.name = 0x2e63007974706d65 <Address 0x2e63007974706d65 out of bounds> So, this is also needed? --- a/tools.c +++ b/tools.c @@ -3624,6 +3624,10 @@ next_arg: ld->end = ld->start + head_member_offset; readmem(ld->start + head_member_offset, KVADDR, &ld->start, sizeof(void *), "LIST_HEAD contents", FAULT_ON_ERROR); + if (ld->start == ld->end) { + fprintf(fp, "(empty)\n"); + return; + }

} else ld->start += ld->list_head_offset; } Thanks, Kazu > + } else > + ld->start += ld->list_head_offset; > + } > } > > ld->flags &= ~(LIST_OFFSET_ENTERED|LIST_START_ENTERED); > -- > 2.31.1