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

HAGIO KAZUHITO(萩尾 一仁) k-hagio-ab at nec.com
Tue May 25 07:32:56 UTC 2021


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 at suse.com>
> ---
>  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





More information about the Crash-utility mailing list