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

----- 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(a)kernel.org&gt; > > 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(a)sr71.net&gt; > > 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/80b49a879e0ba8ce7c365a2fd88... More to come...