----- "Robin Holt" <holt@xxxxxxx> wrote: > On Mon, Apr 27, 2009 at 10:27:22AM -0400, Dave Anderson wrote: > > > > ----- "Michael Holzheu" <holzheu@xxxxxxxxxxxxxxxxxx> 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 -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility