----- Original Message ----- > I was originally thinking about the same as yourself but decided against > it due to thinking first that, in English, QUIET is a bit abstract for > "don't try to switch devmem source" as well as avoid output. > > Plus, if any other readmem() that's QUIET fails READ_ERROR for any > random reason and devmem_is_restricted() will fail a > switch_to_proc_kcore() would have resolved it then that caller's use of > QUIET will prevent devmem_is_restricted() being tried and it won't > switch_to_proc_kcore(). I admit that means the status of > devmem_is_restricted() has changed between initialization and now, but > it's a live environment with it's own behavior. > > Due to the small number of error_handle flags here's one that uses the > best of your idea without expanding the meaning of the word QUIET. It > adds another flag for the sole purpose of saying "don't try to switch > devmem source this time" and it avoids all the overhead I originally had. > > The name for the flag is long enough that the value tab alignment > doesn't match with the other flags (I can't think of a shorter name for > the flag but maybe you can). I chose not to appear to make three extra > lines of changes just to expand the tab alignment on the other flags. It > also leaves PRINT_ERROR_MESSAGE over-riding QUIET as at present but I > don't have a preference for that -d8 has it's useful moments: OK, I'll buy all that -- I like it. I also added an "NDS" string to the error_handle_string() function for the readmem() debug output for "crash -d4" and above. Queued for crash-7.1.4: https://github.com/crash-utility/crash/commit/34842b66a121161a91b328f5eab47abd43661248 Thanks, Dave > > diff --git a/defs.h b/defs.h > index b0cdd42..29f46b4 100644 > --- a/defs.h > +++ b/defs.h > @@ -328,6 +328,7 @@ struct number_option { > #define HEX_BIAS (0x8) > #define LONG_LONG (0x10) > #define RETURN_PARTIAL (0x20) > +#define NO_DEVMEM_SWITCH (0x40) > > #define SEEK_ERROR (-1) > #define READ_ERROR (-2) > diff --git a/memory.c b/memory.c > index 72218e7..b4629a6 100644 > --- a/memory.c > +++ b/memory.c > @@ -2204,14 +2204,14 @@ 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); > - } > - goto readmem_error; > + > + if ((pc->flags & DEVMEM) && (kt->flags & PRE_KERNEL_INIT) && > + !(error_handle & NO_DEVMEM_SWITCH) && devmem_is_restricted() && > switch_to_proc_kcore()) > + return(readmem(addr, memtype, bufptr, size, > + type, error_handle)); > + goto readmem_error; > > case PAGE_EXCLUDED: > RETURN_ON_PARTIAL_READ(); > @@ -2410,16 +2410,16 @@ devmem_is_restricted(void) > if (machine_type("X86") || machine_type("X86_64")) { > if (readmem(255*PAGESIZE(), PHYSADDR, &tmp, > sizeof(long), "devmem_is_allowed - pfn 255", > - QUIET|RETURN_ON_ERROR) && > + QUIET|RETURN_ON_ERROR|NO_DEVMEM_SWITCH) && > !(readmem(257*PAGESIZE(), PHYSADDR, &tmp, > sizeof(long), "devmem_is_allowed - pfn 257", > - QUIET|RETURN_ON_ERROR))) > + QUIET|RETURN_ON_ERROR|NO_DEVMEM_SWITCH))) > restricted = TRUE; > } > if (kernel_symbol_exists("jiffies") && > !readmem(symbol_value("jiffies"), KVADDR, &tmp, > sizeof(ulong), "devmem_is_allowed - jiffies", > - QUIET|RETURN_ON_ERROR)) > + QUIET|RETURN_ON_ERROR|NO_DEVMEM_SWITCH)) > restricted = TRUE; > > if (restricted) > > -- > Crash-utility mailing list > Crash-utility@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
