Re: [Crash-utility] The problems when running SuSE 12 on VirtualBox

On 11/19/2015 12:20 PM, Dave Anderson wrote: > > > ----- Original Message ----- >> On 11/19/2015 08:37 AM, David Mair wrote: >>> On 11/19/2015 07:45 AM, Dave Anderson wrote: >>> >>> Hi Dave, >>> >>>> ----- Original Message ----- >>> <snip> >>>> >>>>> (2) Execute "crash -d8" on physical machine will cause crash utility >>>>> core >>>>> dump. >>>> >>>> I can reproduce this, so I'll look into it. It's related to the >>>> /dev/mem "test" >>>> to determine whether the kernel was configured with >>>> CONFIG_STRICT_DEVMEM, where >>>> it tries to read from pfn 257 (just above the CONFIG_STRICT_DEVMEM >>>> limit), but >>>> gets into an infinite loop when used in conjunction with -d. >>>> >>>> Anyway, just continue to use /proc/kcore and you should be fine. >>> >>> This is the cause in readmem(): >>> >>> switch(READMEM(...)) >>> { >>> . >>> . >>> . >>> case READ_ERROR: >>> if (PRINT_ERROR_MESSAGE) ********** THIS *********** >>> { >>> causes a nested readmem() call before the goto gives it >>> to the caller to deal with >>> } >>> goto readmem_error >>> } >>> . >>> . >>> . >>> switch(error_handle) >>> { >>> case (RETURN_ON_ERROR): >>> } >>> >>> The PRINT_ERROR_MESSAGE I assume is an escalation from -d 8 in this case. >>> >> >> The whole switch_to_proc_kcore() probably shouldn't be conditional on >> the presence of PRINT_ERROR_MESSAGE, only the actual error message >> should be. >> >> Patch shortly. >> >> -- >> David. > > Yeah, I agree. It was done that way to catch the very first non-QUIET > readmem() > call from kernel_init(), and make the bait-and-switch right there. Without > the > CRASHDEBUG(x) override, it works as-is because the readmem() calls in > devmem_is_restricted() are purposely set to QUIET. > > So we're thinking something like this, right?: > > diff --git a/memory.c b/memory.c > index 824b3ae..2282ba9 100644 > --- a/memory.c > +++ b/memory.c > @@ -2207,13 +2207,12 @@ readmem(ulonglong addr, int memtype, void *buffer, > long size, > goto readmem_error; > > case READ_ERROR: > - if (PRINT_ERROR_MESSAGE) { > - if ((pc->flags & DEVMEM) && (kt->flags & > PRE_KERNEL_INIT) && > - devmem_is_restricted() && > switch_to_proc_kcore()) > - return(readmem(addr, memtype, > bufptr, size, > - type, error_handle)); > + if ((pc->flags & DEVMEM) && (kt->flags & > PRE_KERNEL_INIT) && > + !(error_handle & QUIET) && > devmem_is_restricted() && switch_to_proc_kcore()) > + return(readmem(addr, memtype, bufptr, size, > + type, error_handle)); > + if (PRINT_ERROR_MESSAGE) > error(INFO, READ_ERRMSG, > memtype_string(memtype, 0), addr, type); > - } > goto readmem_error; > > case PAGE_EXCLUDED: > > Works for me, and is better than hiding a recursive-call check in > devmem_is_restricted(). That protects from nesting devmem_is_restricted() as long as devmem_is_restricted() continues to do QUIET readmem()s but fails to switch_to_proc_kcore() for QUIET readmem() at other random failing addresses that could be fixed via /proc/kcore. QUIET isn't a global flag only for being in devmem_is_restricted().

Here's mine, it's the kind of clumsy hardcore you were trying to avoid, it's not thread or signal safe. I do feel the same concern you expressed but I believe it does work for all failing QUIET readmem() cases. diff --git a/memory.c b/memory.c index 72218e7..71f1e47 100644 --- a/memory.c +++ b/memory.c @@ -319,6 +319,7 @@ static void dump_hstates(void); #define ASCII_UNLIMITED ((ulong)(-1) >> 1) static ulong DISPLAY_DEFAULT; +static int IN_DEVMEM_IS_RESTRICTED; /* * Verify that the sizeof the primitive types are reasonable. @@ -338,6 +339,7 @@ mem_init(void) error(FATAL, "pointer size: %d is not sizeof(long): %d\n", sizeof(void *), sizeof(long)); DISPLAY_DEFAULT = (sizeof(long) == 8) ? DISPLAY_64 : DISPLAY_32; + IN_DEVMEM_IS_RESTRICTED = FALSE; } @@ -2204,13 +2206,13 @@ readmem(ulonglong addr, int memtype, void *buffer, long size, goto readmem_error; case READ_ERROR: - if (PRINT_ERROR_MESSAGE) { - if ((pc->flags & DEVMEM) && (kt->flags & PRE_KERNEL_INIT) && - devmem_is_restricted() && switch_to_proc_kcore()) - return(readmem(addr, memtype, bufptr, size, - type, error_handle)); + if (PRINT_ERROR_MESSAGE) error(INFO, READ_ERRMSG, memtype_string(memtype, 0), addr, type); - } + + if ((pc->flags & DEVMEM) && (kt->flags & PRE_KERNEL_INIT) && + devmem_is_restricted() && switch_to_proc_kcore()) + return(readmem(addr, memtype, bufptr, size, + type, error_handle)); goto readmem_error; case PAGE_EXCLUDED: @@ -2393,6 +2395,11 @@ devmem_is_restricted(void) long tmp; int restricted; + if (IN_DEVMEM_IS_RESTRICTED) + return FALSE; + IN_DEVMEM_IS_RESTRICTED = TRUE; + + restricted = FALSE; /* * Check for pre-CONFIG_STRICT_DEVMEM kernels. @@ -2401,11 +2408,9 @@ devmem_is_restricted(void) if (machine_type("ARM") || machine_type("ARM64") || machine_type("X86") || machine_type("X86_64") || machine_type("PPC") || machine_type("PPC64")) - return FALSE; + goto end_devmem_is_restricted; } - restricted = FALSE; - if (STREQ(pc->live_memsrc, "/dev/mem")) { if (machine_type("X86") || machine_type("X86_64")) { if (readmem(255*PAGESIZE(), PHYSADDR, &tmp, @@ -2429,6 +2434,9 @@ devmem_is_restricted(void) "source.\n"); } +end_devmem_is_restricted: + IN_DEVMEM_IS_RESTRICTED = FALSE; + return restricted; } -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility