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

Badari Pulavarty pbadari at us.ibm.com
Tue Jan 31 19:38:31 UTC 2006


On Tue, 2006-01-31 at 13:18 -0500, Dave Anderson wrote:
> 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: 
> 

Yep. I just took those changes from Rachita's patch. 
Let me look and get back to you.

Thanks,
Badari




More information about the Crash-utility mailing list