[Crash-utility] [PATCH] cmdline: Add a new "--machdep stacksize=<value>".

Sean Fu fxinrong at gmail.com
Thu Oct 11 04:39:14 UTC 2018


On Wed, Oct 10, 2018 at 09:45:48AM -0400, Dave Anderson wrote:
> 
> 
> ----- Original Message -----
> > On Tue, Oct 09, 2018 at 09:39:10AM -0400, Dave Anderson wrote:
> > > 
> > > 
> > > ----- Original Message -----
> > > > On Mon, Oct 01, 2018 at 09:37:10AM -0400, Dave Anderson wrote:
> > > > > 
> > > > > 
> > > > > ----- Original Message -----
> > > > > > Implemented support for 16k stack size that was introduced by commit
> > > > > > 6538b8ea886e472f4431db8ca1d60478f838d14b titled "x86_64: expand
> > > > > > kernel
> > > > > > stack to 16K".
> > > > > > Without the patch, kernels has 16k stack, leading to errors in
> > > > > > commands
> > > > > > such as "bt" and any command regarding 8K stack.
> > > > > > Add a new "--machdep stacksize=<value>" option that can be used to
> > > > > > override the default machdep->stacksize value which is 8k.
> > > > > 
> > > > > The x86_64 default value of 8K is basically a leftover value that each
> > > > > of
> > > > > the architectures originally used for setting machdep->stacksize.  But
> > > > > for
> > > > > quite some time now, those values should get overridden later on here
> > > > > in task_init():
> > > > > 
> > > > >         STRUCT_SIZE_INIT(task_union, "task_union");
> > > > >         STRUCT_SIZE_INIT(thread_union, "thread_union");
> > > > > 
> > > > >         if (VALID_SIZE(task_union) && (SIZE(task_union) !=
> > > > >         STACKSIZE())) {
> > > > >                 error(WARNING, "\nnon-standard stack size: %ld\n",
> > > > >                         len = SIZE(task_union));
> > > > >                 machdep->stacksize = len;
> > > > >         } else if (VALID_SIZE(thread_union) &&
> > > > >                 ((len = SIZE(thread_union)) != STACKSIZE())) {
> > > > >                 machdep->stacksize = len;
> > > > >         } else if (!VALID_SIZE(thread_union) &&
> > > > >         !VALID_SIZE(task_union)) {
> > > > >                 if (kernel_symbol_exists("__start_init_task") &&
> > > > >                     kernel_symbol_exists("__end_init_task")) {
> > > > >                         len = symbol_value("__end_init_task");
> > > > >                         len -= symbol_value("__start_init_task");
> > > > >                         ASSIGN_SIZE(thread_union) = len;
> > > > >                         machdep->stacksize = len;
> > > > >                 }
> > > > >         }
> > > > > 
> > > > I compiled latest kernel and latest crash and run a qemu guest machine
> > > > with
> > > > the latest compliled kernel
> > > > image.
> > > > In this case, STRUCT_SIZE_INIT initialized size_table.task_union and
> > > > size_table.thread_union with -1. So machdep->stacksize did NOT get
> > > > overridden.
> > > > > As of Linux 4.18 at least, x86_64 still uses the thread_union
> > > > > declaration.
> > > > > For example:
> > > > > 
> > > > >   crash> thread_union
> > > > >   union thread_union {
> > > > >       struct task_struct task;
> > > > >       unsigned long stack[2048];
> > > > >   }
> > > > >   SIZE: 16384
> > > > >   crash>
> > > > > 
> > > > > On what kernel version are you seeing the obsolete 8k stacksize being
> > > > > used?
> > > > > What does the command above show on your system?
> > > > kernel version is upstream Linux 4.18
> > > > (commit#94710cac0ef4ee177a63b5227664b38c95bbf703)
> > > > (git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git).
> > > > 
> > > > "bt" command in crash shows "bt: invalid RSP: ffffc9000069bc08
> > > > bt->stackbase/stacktop: ffffc90000698000/ffffc9000069a000 cpu: 0".
> > > > 
> > > > BestRegards
> > > > Sean
> > > 
> > > 
> > > 
> > > Ok, the most recent 4.18 kernel I have on hand is this one:
> > > 
> > >   crash> sys | grep RELEASE
> > >        RELEASE: 4.18.0-20.el8.x86_64
> > >   crash>
> > > 
> > > and its debuginfo data contains the "thread_union" information:
> > > 
> > >   crash> thread_union
> > >   union thread_union {
> > >       struct task_struct task;
> > >       unsigned long stack[2048];
> > >   }
> > >   SIZE: 16384
> > >   crash>
> > > 
> > > but if it did not, then code should then calculate the stack
> > > size from the difference between the "__start_init_task" and
> > > "__end_init_task" symbols:
> > > 
> > >   crash> sym __start_init_task
> > >   ffffffffa7800000 (D) __start_init_task
> > >   crash> sym __end_init_task
> > >   ffffffffa7804000 (D) __end_init_task
> > >   crash>
> > >   
> > > Does your kernel not show/contain those 2 symbols?
> > >
> > Sure, my test kernel contains these 2 symbols.
> > 
> > crash> sys | grep RELEASE
> >      RELEASE: 4.18.0-1-default+
> > crash> thread_union
> > crash: command not found: thread_union
> > crash> struct thread_union
> > struct: invalid data structure reference: thread_union
> > crash> sym __start_init_task
> > ffffffff82000000 (D) __start_init_task
> > crash> sym __end_init_task
> > ffffffff82004000 (D) __end_init_task
> > 
> > Agree with you, Automatic calculation stack size from  the difference
> > between__start_init_task and __end_init_task should be better.
> > Calculating and assignning stack size should be add into "x86_64_init", Do
> > you think so?
>  
> No, because the calculation is being done in an architecture-neutral manner
> by task_init(), here in task.c:
> 
>     437         if (VALID_SIZE(task_union) && (SIZE(task_union) != STACKSIZE())) {
>     438                 error(WARNING, "\nnon-standard stack size: %ld\n",
>     439                         len = SIZE(task_union));
>     440                 machdep->stacksize = len;
>     441         } else if (VALID_SIZE(thread_union) &&
>     442                 ((len = SIZE(thread_union)) != STACKSIZE())) {
>     443                 machdep->stacksize = len;
>     444         } else if (!VALID_SIZE(thread_union) && !VALID_SIZE(task_union)) {
>     445                 if (kernel_symbol_exists("__start_init_task") &&
>     446                     kernel_symbol_exists("__end_init_task")) {
>     447                         len = symbol_value("__end_init_task");
>     448                         len -= symbol_value("__start_init_task");
>     449                         ASSIGN_SIZE(thread_union) = len;
>     450                         machdep->stacksize = len;
>     451                 }
>     452         }
> 
> I see that "thread_union" is not found in your debuginfo data, but I don't understand
> how your kernel gets past the second "else" segment above where the __start_init_task
> and __end_init_task symbol values are checked.
Your code is different from mine. The following is from my task.c:

  436         if (VALID_SIZE(task_union) && (SIZE(task_union) != STACKSIZE())) {
  437                 error(WARNING, "\nnon-standard stack size: %ld\n",
  438                         len = SIZE(task_union));
  439                 machdep->stacksize = len;
  440         } else if (VALID_SIZE(thread_union) &&
  441                 ((len = SIZE(thread_union)) != STACKSIZE()))
  442                 machdep->stacksize = len;
  443 
  444         MEMBER_OFFSET_INIT(pid_namespace_idr, "pid_namespace", "idr");
  445         MEMBER_OFFSET_INIT(idr_idr_rt, "idr", "idr_rt");

My code repo:
sean at linux-zmni:~/work/source/upstream/crash> git remote -v
origin	https://github.com/crash-utility/crash.git (fetch)
origin	https://github.com/crash-utility/crash.git (push)

What's your crash version?

Thanks
Sean
> 
> Dave
> 
> 




More information about the Crash-utility mailing list