[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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




----- Original Message -----
> 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 suse cz>

Cool -- queued for the next release!

BTW, I appreciate the Xen version 4 support.  Given that Red Hat has dropped
Xen as of RHEL6, there wouldn't be any ongoing maintenance coming from this
side of the house.

Thanks again,
  Dave


> ---
> 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;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]