[Crash-utility] [PATCH] Support cpu_map/cpu_mask changes in 2.6.29

[Michael Holzheu]

Hi Dave,

Am Montag, den 27.04.2009, 11:58 -0400 schrieb Dave Anderson:
> ----- "Robin Holt" <holt at sgi.com> wrote:
> 
> > On Mon, Apr 27, 2009 at 10:27:22AM -0400, Dave Anderson wrote:
> > > 
> > > ----- "Michael Holzheu" <holzheu at linux.vnet.ibm.com> wrote:
> > > 
> > > > > So yes, while STRUCT_SIZE("cpumask_t") would always be appropriate for that
> > > > > data type, it would fail for the older kernel types which don't use it.
> > > > 
> > > > So if for older kernels it was an unsigned long, the function should
> > > > work:
> > > > 
> > > > +static int
> > > > +cpu_map_size(void)
> > > > +{
> > > > +	int len;
> > > > +
> > > > +	len = STRUCT_SIZE("cpumask_t");
> > > > +	if (len < 0)
> > > > +		return sizeof(ulong);
> > > > +	else
> > > > +		return len;
> > > > +}
> > > 
> > > Yeah, you're right, that will probably work.  There were definitions for
> > > "cpumask_t" that existed prior to the transition of cpu_online_map symbol
> > > from being an unsigned long to a cpumask_t.  But except for mips64 (unsupported)
> > > they all appear to have been unsigned longs anyway.
> > 
> > IIRC, cpumask_t on ia64 hasn't been an unsigned long for a very long time.
> > The generic_defconfig was at 512 until it recently jumped to 2048.
> > Only some configs limited it down to an unsigned long.  Unfortunately,
> > I don't have much time to test this week.  Maybe next, but a quick code
> > inspection should raise flags if I am remembering correctly.
> > 
> > Thanks,
> > Robin
> 
> OK, so then there are two functions in Michael's patch that are
> primarily relevant to maintaining backwards compatibility, cpu_map_addr()
> and cpu_map_size().
> 
> cpu_map_addr() basically replaces the hardwired usage of the relevant
> symbol address as an argument to kernel_symbol_exists("cpu_xxxx_map").
> But when the pre-2.6.29 "cpu_xxxx_map" symbols exist, the code still
> does the right thing:
> 
> +ulong
> +cpu_map_addr(const char *type)
> +{
> +       char cpu_map_symbol[32];
> +
> +       sprintf(cpu_map_symbol, "cpu_%s_map", type);
> +       if (symbol_exists(cpu_map_symbol))
> +               return symbol_value(cpu_map_symbol);
> +       else
> +               return get_cpu_map_addr_from_mask(type);
> +}
> 
> Although, for safety's sake above, the symbol_value() call should
> be replaced with kernel_symbol_value() in order to rule out the
> invalid usage of stray module symbols of the same name.
> 
> The cpu_map_size() function is the real bone of contention:
> 
> +static int
> +cpu_map_size(void)
> +{
> +       int len;
> +
> +       len = STRUCT_SIZE("cpumask_t");
> +       if (len < 0)
> +               return sizeof(ulong);
> +       else
> +               return len;
> +}
> 
> because it is used like so:
> 
> -       if (LKCD_KERNTYPES()) {
> -               if ((len = STRUCT_SIZE("cpumask_t")) < 0)
> -                       error(FATAL, "cannot determine type cpumask_t\n");
> -       } else
> -               len = get_symbol_type("cpu_online_map", NULL, &req) ==
> -                       TYPE_CODE_UNDEF ?  sizeof(ulong) : req.length;
> +       len = cpu_map_size();
> 
> So to cover all bases, perhaps cpu_map_size() should still incorporate
> the get_symbol_type() mechanism *if* the "cpu_xxxx_map" symbol still
> exists?
> 
> Michael, does that make sense?

Yes, this makes sense. Will you change the patch accordingly?

Michael