On Thu, Jun 24, 2021 at 11:02 AM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> wrote:
>
> -----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");
After the log level is set to FATAL, it will exit, and the "return"
has no chance to execute . So it could be good
to use the log level "WARNING or INFO" for error()?
Thanks.
Lianbo
> + 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
