On Mon, Jun 21, 2021 at 7:06 PM HAGIO KAZUHITO(萩尾 一仁) <[email protected]>
wrote:

> Hi Greg,
>
> I'm resending with cc, my email client dropped it automatically..
>
> (We would recommend subscribing to the mailing list so that you can receive
> replies certainly.)
>
> Thanks,
> Kazu
>
> -----Original Message-----
> > Hi Greg,
> >
> > -----Original Message-----
> > > The wait queue structs and members were renamed in 4.13.  Add support
> to
> > > the 'waitq' command for these more recent kernels.
> >
> > Thanks for the patch, good catch.
> >
> > (Ah, my test has missed the command..)
> >
> > It would be helpful if you add the kernel commits to the commit log,
> > I think they are ac6424b981bce1c4bc55675c6ce11bfe1bbfa64f and
> > 9d9d676f595b5081326be7a17dc681fcb38fb3b2.
> >
> > >
> > > Signed-off-by: Greg Edwards <[email protected]>
> > > ---
> > >  defs.h    |  4 ++++
> > >  kernel.c  | 23 +++++++++++++++++++++--
> > >  symbols.c |  8 +++++++-
> > >  3 files changed, 32 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/defs.h b/defs.h
> > > index 42c8074e6ac6..170e9cba724f 100644
> > > --- a/defs.h
> > > +++ b/defs.h
> > > @@ -1589,6 +1589,9 @@ struct offset_table {                    /*
> stash of commonly-used offsets */
> > >     long __wait_queue_task;
> > >     long __wait_queue_head_task_list;
> > >     long __wait_queue_task_list;
> > > +   long wait_queue_entry_private;
> > > +   long wait_queue_head_head;
> > > +   long wait_queue_entry_entry;
> > >     long pglist_data_node_zones;
> > >     long pglist_data_node_mem_map;
> > >     long pglist_data_node_start_paddr;
> > > @@ -2188,6 +2191,7 @@ struct size_table {         /* stash of
> commonly-👍used sizes */
> > >     long tvec_t_base_s;
> > >     long wait_queue;
> > >     long __wait_queue;
> > > +   long wait_queue_entry;
> >
> > Would you move these offsets and size to the end of its table?
> > to avoid breaking previously compiled extension modules.
> >
> > (ref. crash's contribution guidelines:
> https://github.com/crash-utility/crash/wiki)
> >
> >
> > >     long device;
> > >     long net_device;
> > >     long sock;
> > > diff --git a/kernel.c b/kernel.c
> > > index 528f6ee524f6..1d50a7200e42 100644
> > > --- a/kernel.c
> > > +++ b/kernel.c
> > > @@ -614,8 +614,16 @@ kernel_init()
> > >     } else if (symbol_exists("tvec_bases"))
> > >             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)) {
> > > +           STRUCT_SIZE_INIT(__wait_queue, "__wait_queue");
> >
> > The VALID_STRUCT(__wait_queue) macro can be used after calling
> > STRUCT_SIZE_INIT(__wait_queue, "__wait_queue"), so please move this
> > prior to the if block.
> >
> > >             if (MEMBER_EXISTS("__wait_queue", "task"))
> > >                     MEMBER_OFFSET_INIT(__wait_queue_task,
> > >                             "__wait_queue", "task");
> > > @@ -9397,6 +9405,17 @@ 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 task_list_offset;
> > > +
> > > +                next_offset = OFFSET(list_head_next);
> > > +                task_offset = OFFSET(wait_queue_entry_private);
> > > +                task_list_offset = OFFSET(wait_queue_head_head);
> > > +                ld->end = ld->start = wq + task_list_offset +
> next_offset;
> > > +                ld->list_head_offset = OFFSET(wait_queue_entry_entry);
> > > +                ld->member_offset = next_offset;
> > > +
> > >             start_index = 1;
> > >     } else {
> > >             return;
> >
> > Personally I would like to add a message to inform the structure change
> > so that my test can detect it.  So could you replace the return with
> this?
> >
> >   error(FATAL, "cannot determine wait queue structure\n");
> >
> >
> > > diff --git a/symbols.c b/symbols.c
> > > index 370d4c3e8ac0..72eea43871bb 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));
> > > +
> >
> > Please print also SIZE(wait_queue_entry).
> >
> > Thanks,
> > Kazu
> >
> > >     fprintf(fp, "        pglist_data_node_zones: %ld\n",
> > >             OFFSET(pglist_data_node_zones));
> > >     fprintf(fp, "      pglist_data_node_mem_map: %ld\n",
> > > --
> > > 2.32.0
> > >
> > >
> > > --
> > > Crash-utility mailing list
> > > [email protected]
> > > https://listman.redhat.com/mailman/listinfo/crash-utility
> >
> >
> > --
> > Crash-utility mailing list
> > [email protected]
> > https://listman.redhat.com/mailman/listinfo/crash-utility
>
>
> --
> Crash-utility mailing list
> [email protected]
> https://listman.redhat.com/mailman/listinfo/crash-utility
>
> --
Nick Santucci

[email protected]
--
Crash-utility mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/crash-utility

Reply via email to