[Crash-utility] Support for dynamic allocations of 'struct fpu'

[Dave Anderson]

----- Original Message -----
> 
> 
> ----- Original Message -----
> > Hello,
> > 
> > This is a follow-up report of the issue I found in the release test
> > for makedumpfile-1.5.9. The original report was posted in kexec-ML:
> > 
> >   http://lists.infradead.org/pipermail/kexec/2015-October/014620.html
> > 
> > > o Support new kernels
> > >  - The supported kernel is updated to 4.1 in this version.
> > >    At first I'm going to extend the supported version to 4.2, but
> > >    I found an issue that makedumpfile seems to exclude necessary pages
> > >    by mistake on linux 4.2. When crash-7.1.3 reads that filtered dump
> > >    file,
> > >    the following message is shown.
> > > 
> > >       crash: page excluded: kernel virtual address: f3fe0000  type:
> > >       "fill_task_struct"
> > > 
> > >    This will be fixed in the next version.
> > 
> > I looked for the cause of this issue and found that it doesn't seem to be
> > an
> > issue of makedumpfile, it seems to be a crash side issue.
> > 
> > The size of task_struct is decided dynamically by FPU registers since linux
> > 4.2
> > due to:
> > 
> >   commit 5aaeb5c01c5b6c0be7b7aadbf3ace9f3a4458c3d
> >   Author: Ingo Molnar <mingo at kernel.org>
> >   Date:   Fri Jul 17 12:28:12 2015 +0200
> > 
> >       x86/fpu, sched: Introduce CONFIG_ARCH_WANTS_DYNAMIC_TASK_STRUCT and
> >       use
> >       it on x86
> > 
> > and
> > 
> >   commit 0c8c0f03e3a292e031596484275c14cf39c0ab7a
> >   Author: Dave Hansen <dave at sr71.net>
> >   Date:   Fri Jul 17 12:28:11 2015 +0200
> > 
> >       86/fpu, sched: Dynamically allocate 'struct fpu'
> > 
> > This change will cause a difference between dwarf info and the actual size
> > like below:
> > (This is an example in linux 4.2 on x86_64)
> > 
> > - dwarf info
> > 
> >     $ dwarfdump vmlinux | grep -A 2 task_struct
> >     DW_AT_name                  "task_struct"
> >     DW_AT_byte_size             0x00001940                         // 6464
> >     byte
> >     DW_AT_decl_file             0x0000001a include/linux/sched.h
> > 
> > - actual size
> > 
> >     crash> p arch_task_struct_size
> >     arch_task_struct_size = $1 = 2880
> >     crash>
> > 
> > I don't think crash handle this change, so crash can read an irrelevant
> > page when trying to read a task_struct. If the dump is filtered by
> > makedumpfile
> > and the page just behind the task_struct is excluded, the message I
> > reported
> > will be shown.
> > 
> > To fix the size_table for the crash's initialization is easy, we should
> > just update it by arch_task_struct_size like:
> > 
> > 
> > diff --git a/task.c b/task.c
> > index 8956fb5..ee94d4e 100755
> > --- a/task.c
> > +++ b/task.c
> > @@ -284,6 +284,17 @@ task_init(void)
> >         MEMBER_OFFSET_INIT(pid_pid_chain, "pid", "pid_chain");
> > 
> >         STRUCT_SIZE_INIT(task_struct, "task_struct");
> > +       int task_struct_size;
> > +       if (kernel_symbol_exists("arch_task_struct_size") &&
> > +           readmem(symbol_value("arch_task_struct_size"), KVADDR,
> > +                   &task_struct_size, sizeof(int),
> > +                   "arch_task_struct_size", RETURN_ON_ERROR)) {
> > +               ASSIGN_SIZE(task_struct) = task_struct_size;
> > +               if (CRASHDEBUG(1))
> > +                       fprintf(fp, "\downsize_task_struct: %ld to %ld\n",
> > +                               STRUCT_SIZE("task_struct"),
> > +                               SIZE(task_struct));
> > +       }
> > 
> > 
> > However, struct command always refer to dwarf info since it's probably
> > designed for general purpose, I can't come up with good way to fix it.
> > 
> > Do you have any comments ?
> > 
> > Thanks,
> > Atsushi Kumagai
> 
> Hi Atsushi,
> 
> Thanks for reporting this -- I missed seeing that patch go into the
> upstream kernel.
> 
> At a minimum your patch above will certainly be necessary.  But I don't
> know any way to work around the fact that gdb/crash will be bound to the
> debuginfo data as far as displaying the contents, for example, when
> a user manually creates a gdb print command.  In the case of kdump,
> dumpfiles, --zero_excluded would either have to be set on the command line
> or "set" during the crash session.  And I suppose I can flag attempts to
> access bogus appended data with the "struct" or "task" commands, and
> perhaps just fill the remainder with zeroes.
> 
> It's going to be ugly.  What a pain in the ass...
> 
> Thanks,
>   Dave

The patch above has been queued for crash-7.1.4:

  https://github.com/crash-utility/crash/commit/80b49a879e0ba8ce7c365a2fd88693128a79310f

More to come...

Thanks,
  Dave
 

> 
> 
> 
> 
> 
>    
>