Re: [PATCH v2] Fix 'waitq' command for 4.13 and later kernels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----Original Message-----
> The wait queue structs and members were renamed in 4.13 in commits:
> 
>   ac6424b981bc ("sched/wait: Rename wait_queue_t => wait_queue_entry_t")
>   9d9d676f595b ("sched/wait: Standardize internal naming of wait-queue heads")
>   2055da97389a ("sched/wait: Disambiguate wq_entry->task_list and wq_head->task_list naming")
> 
> Add support to the 'waitq' command for these more recent kernels.
> 
> Signed-off-by: Greg Edwards <gedwards@xxxxxxx>
> ---
> Changes from v1:
>   * address Kazu's review feedback

Thanks for the update.

> 
>  defs.h    |  4 ++++
>  kernel.c  | 23 +++++++++++++++++++++--
>  symbols.c | 10 +++++++++-
>  3 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/defs.h b/defs.h
> index 42c8074e6ac6..6bb00e29d811 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -2138,6 +2138,9 @@ struct offset_table {                    /* stash of commonly-used offsets */
>  	long atomic_long_t_counter;
>  	long block_device_bd_device;
>  	long block_device_bd_stats;
> +	long wait_queue_entry_private;
> +	long wait_queue_head_head;
> +	long wait_queue_entry_entry;
>  };
> 
>  struct size_table {         /* stash of commonly-used sizes */
> @@ -2300,6 +2303,7 @@ struct size_table {         /* stash of commonly-used sizes */
>  	long printk_info;
>  	long printk_ringbuffer;
>  	long prb_desc;
> +	long wait_queue_entry;
>  };
> 
>  struct array_table {
> diff --git a/kernel.c b/kernel.c
> index 528f6ee524f6..6a379229713d 100644
> --- a/kernel.c
> +++ b/kernel.c
> @@ -615,7 +615,15 @@ kernel_init()
>  		kt->flags |= TVEC_BASES_V1;
> 
>          STRUCT_SIZE_INIT(__wait_queue, "__wait_queue");
> -        if (VALID_STRUCT(__wait_queue)) {
> +	STRUCT_SIZE_INIT(wait_queue_entry, "wait_queue_entry");
> +	if (VALID_STRUCT(wait_queue_entry)) {
> +		MEMBER_OFFSET_INIT(wait_queue_entry_private,
> +			"wait_queue_entry", "private");
> +		MEMBER_OFFSET_INIT(wait_queue_head_head,
> +			"wait_queue_head", "head");
> +		MEMBER_OFFSET_INIT(wait_queue_entry_entry,
> +			"wait_queue_entry", "entry");
> +	} else if (VALID_STRUCT(__wait_queue)) {
>  		if (MEMBER_EXISTS("__wait_queue", "task"))
>  			MEMBER_OFFSET_INIT(__wait_queue_task,
>  				"__wait_queue", "task");
> @@ -9397,9 +9405,20 @@ dump_waitq(ulong wq, char *wq_name)
>                  ld->list_head_offset = OFFSET(__wait_queue_task_list);
>                  ld->member_offset = next_offset;
> 
> +		start_index = 1;
> +	} else if (VALID_STRUCT(wait_queue_entry)) {
> +		ulong head_offset;
> +
> +		next_offset = OFFSET(list_head_next);
> +		task_offset = OFFSET(wait_queue_entry_private);
> +		head_offset = OFFSET(wait_queue_head_head);
> +		ld->end = ld->start = wq + head_offset + next_offset;
> +		ld->list_head_offset = OFFSET(wait_queue_entry_entry);
> +		ld->member_offset = next_offset;
> +
>  		start_index = 1;
>  	} else {
> -		return;
> +		error(FATAL, "cannot determine wait queue structure\n");

oh, I should have checked the replacement.. this emits compilation warnings:

$ make clean ; make warn
...
cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6  kernel.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security 
kernel.c: In function 'cmd_waitq':
kernel.c:9380:6: warning: 'start_index' may be used uninitialized in this function [-Wmaybe-uninitialized]
  int start_index;  /* where to start in wq array */
      ^~~~~~~~~~~
kernel.c:9454:22: warning: 'task_offset' may be used uninitialized in this function [-Wmaybe-uninitialized]
   readmem(wq_list[i] + task_offset, KVADDR, &task,
           ~~~~~~~~~~~^~~~~~~~~~~~~
kernel.c:9378:8: note: 'task_offset' was declared here
  ulong task_offset;  /* offset of task in wq element */
        ^~~~~~~~~~~

Hmm, in this case, I'd like to put the return back when merging.
Lianbo, is this ok?

        } else {
                error(FATAL, "cannot determine wait queue structure\n");
+               return; /* just to suppress useless compilation warnings */
        }

Otherwise, the patch looks good to me.

Acked-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>

Thanks,
Kazu


>  	}
> 
>  	hq_open();
> diff --git a/symbols.c b/symbols.c
> index 370d4c3e8ac0..bdc1cac98348 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -9817,7 +9817,13 @@ dump_offset_table(char *spec, ulong makestruct)
>          	OFFSET(__wait_queue_head_task_list));
>          fprintf(fp, "        __wait_queue_task_list: %ld\n",
>          	OFFSET(__wait_queue_task_list));
> -
> +	fprintf(fp, "        wait_queue_entry_private: %ld\n",
> +		OFFSET(wait_queue_entry_private));
> +	fprintf(fp, "        wait_queue_head_head: %ld\n",
> +		OFFSET(wait_queue_head_head));
> +	fprintf(fp, "        wait_queue_entry_entry: %ld\n",
> +		OFFSET(wait_queue_entry_entry));
> +
>  	fprintf(fp, "        pglist_data_node_zones: %ld\n",
>  		OFFSET(pglist_data_node_zones));
>  	fprintf(fp, "      pglist_data_node_mem_map: %ld\n",
> @@ -10717,6 +10723,8 @@ dump_offset_table(char *spec, ulong makestruct)
>  	fprintf(fp, "                    wait_queue: %ld\n", SIZE(wait_queue));
>  	fprintf(fp, "                  __wait_queue: %ld\n",
>  		SIZE(__wait_queue));
> +	fprintf(fp, "              wait_queue_entry: %ld\n",
> +		SIZE(wait_queue_entry));
>  	fprintf(fp, "                        device: %ld\n", SIZE(device));
>  	fprintf(fp, "                    net_device: %ld\n", SIZE(net_device));
> 
> --
> 2.32.0


--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux