Re: [Crash-utility] [PATCH] Support cpu_map/cpu_mask changes in 2.6.29
----- "Robin Holt" <holt(a)sgi.com> wrote:
On Mon, Apr 27, 2009 at 10:27:22AM -0400, Dave Anderson wrote:
>
> ----- "Michael Holzheu" <holzheu(a)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?
Dave