Badari Pulavarty wrote:
 On Wed, 2006-02-01 at 21:05 +0530, Rachita Kothiyal wrote:
 > On Wed, Feb 01, 2006 at 08:47:07AM -0500, Dave Anderson wrote:
 > >
 > > That should do it.
 > >
 > > Minor nits:  for clarity's sake, I'd make the local variable have the
 > > same name as the kernel symbol it's representing (i.e. _cpu_data
 > > instead of __cpu_data), just check it as a boolean instead of it
 > > being == 1, and increment cpu_pda by sizeof(void *).
 >
 > Hi Dave
 >
 > Incorporating the suggested changes and resending the
 > patch. Kindly review.
 >
 > Thanks
 > Rachita
 >
 > Signed-off-by: Rachita Kothiyal <rachita(a)in.ibm.com>
 > ---
 >
 >  defs.h   |    9 +++++++
 >  x86_64.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++----------------
 >  2 files changed, 67 insertions(+), 19 deletions(-)
 >
 > diff -puN x86_64.c~crash-fix-cpu-pda x86_64.c
 > --- crash-4.0-2.19/x86_64.c~crash-fix-cpu-pda 2006-02-01 20:08:14.000000000 +0530
 > +++ crash-4.0-2.19-rachita/x86_64.c   2006-02-01 20:59:59.033113672 +0530
 > @@ -365,9 +365,9 @@ x86_64_dump_machdep_table(ulong arg)
 >  static void
 >  x86_64_cpu_pda_init(void)
 >  {
 > -     int i, cpus, nr_pda, cpunumber;
 > +     int i, cpus, nr_pda, cpunumber, _cpu_pda;
 >       char *cpu_pda_buf;
 > -     ulong level4_pgt, data_offset;
 > +     ulong level4_pgt, data_offset, cpu_pda_addr;
 >       struct syment *sp, *nsp;
 >       ulong offset, istacksize;
 >
 > @@ -383,12 +383,26 @@ x86_64_cpu_pda_init(void)
 >
 >       cpu_pda_buf = GETBUF(SIZE(x8664_pda));
 >
 > -     if (!(nr_pda = get_array_length("cpu_pda", NULL, 0)))
 > -             nr_pda = NR_CPUS;
 > +     if (symbol_exists("_cpu_pda")) {
 > +             if (!(nr_pda = get_array_length("_cpu_pda", NULL, 0)))
 > +                     nr_pda = NR_CPUS;
 > +             _cpu_pda = TRUE;
 > +     } else {
 > +             if (!(nr_pda = get_array_length("cpu_pda", NULL, 0)))
 > +                     nr_pda = NR_CPUS;
 > +             _cpu_pda = FALSE;
 > +     }
 >
 >       for (i = cpus = 0; i < nr_pda; i++) {
 > -             if (!CPU_PDA_READ(i, cpu_pda_buf))
 > -                     break;
 > +             if (_cpu_pda) {
 > +                     cpu_pda_addr = 0;
 > +                     if (!_CPU_PDA_READ(i, cpu_pda_buf))
 > +                             break;
 Small nit. Why do we initialize "cpu_pda_addr = 0" ? Anyway, it will
 be set by _CPU_PDA_READ() isn't it ? Is it to catch, readmem() failure ?
 Thanks,
 Badari 
It's unnecessary in all locations, since _CPU_PDA_READ() initializes
its contents with readmem(FAULT_ON_ERROR).  If that happens,
the crash session will abort during initialization.
But, aside from that, this last patch looks good and tests OK on
pre-2.6.16 kernels.
Dave