Re: [Crash-utility] [PATCH] Make the __per_cpu_offset symbol available

On Friday 07 of January 2011 16:20:15 Dave Anderson wrote: > ----- Original Message ----- > > > Hi, > > > > the code in xen_hyper_init() tries to determine the per-cpu shift from > > the __per_cpu_shift symbol if available. This doesn't work because the > > symbol's value is outside the kernel text address range. This patch makes > > a special case for this symbol on Xen targets, so it can be used > > later. > > Hi Petr, Hi Dave, > The patch looks OK -- but it seems to be a bit of overkill adding a bunch > of new functions? Couldn't each of the original functions simply have > a check like this right at the top?: > > if (XEN_HYPER_MODE() && STREQ(symbol, "__per_cpu_shift")) > return TRUE; Er, yes. When I started the patch, I thought I would need some more special handling, but it then boiled down to just this one symbol, so you're right, it works. > > Without this patch, crash initialization fails on SLES11 Xen hypervisor > > dumps, because on incorrect per-cpu offset is assumed. The failure can't > > be fixed simply by adding a version check, because SLES11 Xen is patched > > to increase the per-cpu shift in order to support more CPUs. > > > > It is also important not to try to relocate ABSOLUTE symbol (such as this > > one), so add this condition in store_symbols(). > > The reloc stuff is only applicable to live x86/x86_64 vmlinux sessions. To > absolutely ensure backwards-compatibility, I'd rather just put this at the > top of relocate(): > > if (XEN_HYPER_MODE()) > return symval; > > Would that work for you? Yes, sure. As long as we don't have live sessions on relocatable Xen hypervisors. Which isn't coming... Here's the shorter patch: Signed-off-by: Petr Tesarik <ptesarik(a)suse.cz&gt;

--- ia64.c | 3 +++ symbols.c | 3 +++ x86.c | 3 +++ x86_64.c | 3 +++ 4 files changed, 12 insertions(+) --- a/ia64.c +++ b/ia64.c @@ -711,6 +711,9 @@ ia64_verify_symbol(const char *name, ulo if (!name || !strlen(name)) return FALSE; + if (XEN_HYPER_MODE() && STREQ(name, "__per_cpu_shift")) + return TRUE; + if (CRASHDEBUG(8)) fprintf(fp, "%016lx %s\n", value, name); --- a/symbols.c +++ b/symbols.c @@ -678,6 +678,9 @@ store_sysmap_symbols(void) static ulong relocate(ulong symval, char *symname, int first_symbol) { + if (XEN_HYPER_MODE()) + return symval; + switch (kt->flags & (RELOC_SET|RELOC_FORCE)) { case RELOC_SET: --- a/x86.c +++ b/x86.c @@ -3785,6 +3785,9 @@ x86_is_task_addr(ulong task) static int x86_verify_symbol(const char *name, ulong value, char type) { + if (XEN_HYPER_MODE() && STREQ(name, "__per_cpu_shift")) + return TRUE; + if (CRASHDEBUG(8) && name && strlen(name)) fprintf(fp, "%08lx %s\n", value, name); --- a/x86_64.c +++ b/x86_64.c @@ -1989,6 +1989,9 @@ x86_64_verify_symbol(const char *name, u if (!name || !strlen(name)) return FALSE; + if (XEN_HYPER_MODE() && STREQ(name, "__per_cpu_shift")) + return TRUE; + if (!(machdep->flags & KSYMS_START)) { if (STREQ(name, "_text") || STREQ(name, "_stext")) { machdep->flags |= KSYMS_START;