Re: [Crash-utility] Re: [RFC]Patch to accomodate cpu_pda changesin x86_64 kernels

Badari Pulavarty wrote: > On Tue, 2006-01-31 at 21:38 +0530, Rachita Kothiyal wrote: > > On Tue, Jan 31, 2006 at 09:11:36AM -0500, Dave Anderson wrote: > > > > > > Thanks for handling this. I haven't looked at this, and correct > me > > > if I'm wrong, but the kernel (what version exactly?) now has > contains > > > a _cpu_pda[NR_CPUS] array containing pointers to the actual per- > cpu > > > x8664_pda structures. > > > > Hi Dave, > > > > This change was introduced 2.6.16-rc1 onwards. > > > > > > Unless I'm missing something, that simplifies things and should > work > > > just fine. But I can't test it here other than to verify that > it's > > > backwards compatible. Can you verify that? > > > > Yes, this is infact more simpler and cleaner. I have incorporated > the > > changes and regenerated the patch, which I am sending along. I > have > > also tested it on a 2.6.16-rc1 kernel, and it works fine. > > > > Thanks > > Rachita > > Hmm.. Lots of duplicated code. I hacked the same thing earlier to > make > it work. I incorporated my changes into yours. > > Does this look better ? > > (BTW, is there a way to combine CPU_PDA_READ() and _CPU_PDA_READ() > by > abstracting some of it out ?) > > Thanks, > Badari > > Rachita, Badari, I think I agree about the unnecessary duplication of code in x86_64_cpu_pda_init() and x86_64_get_smp_cpus(). And while abstracting some of it out to CPU_PDA_READ() may be desirable, keeping them separate may make future maintenance and/or understandability of the differences may make it worth keeping them separate. But I don't have a preference either way. However, upon further review, I don't see how this piece of the patch can work for x86_64_display_cpu_data(): @@ -2828,7 +2855,10 @@ x86_64_display_cpu_data(void) boot_cpu = TRUE; cpus = 1; } q - cpu_pda = symbol_value("cpu_pda"); + if (symbol_exists("_cpu_data")) + cpu_pda = symbol_value("_cpu_pda"); + else if (symbol_exists("cpu_data")) + cpu_pda = symbol_value("cpu_pda"); for (cpu = 0; cpu < cpus; cpu++) { if (boot_cpu) because here is how "cpu_data" is used later on in the function: