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

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?