Messages in this thread Patch in this message | | | Subject | Re: [PATCH 4/8] x86/platform/uv: Setup UV functions for Hubless UV Systems | From | Mike Travis <> | Date | Wed, 11 Sep 2019 13:58:45 -0700 |
| |
On 9/11/2019 1:44 PM, Mike Travis wrote: > > > On 9/10/2019 11:07 PM, Ingo Molnar wrote: >> >> * Mike Travis <mike.travis@hpe.com> wrote: >> >>> +/* Initialize UV hubless systems */ >>> +static __init int uv_system_init_hubless(void) >>> +{ >>> + int rc; >>> + >>> + /* Setup PCH NMI handler */ >>> + uv_nmi_setup_hubless(); >>> + >>> + /* Init kernel/BIOS interface */ >>> + rc = uv_bios_init(); >>> + >>> + return rc; >>> +} > > This looks like an excessive cleanup error by me. The original was: > >> +static __init int uv_system_init_hubless(void) >> +{ >> + int rc; >> + >> + /* Setup PCH NMI handler */ >> + uv_nmi_setup_hubless(); >> + >> + /* Init kernel/BIOS interface */ >> + rc = uv_bios_init(); >> + >> + /* Create user access node if UVsystab available */ >> + if (rc >= 0) >> + uv_setup_proc_files(1); >> + >> + return rc; >> +} >> + > > Hubbed UV's do not have a non-UV BIOS, but hubless systems in theory > can. So uv_bios_init can fail on hubless systems if it has some other > BIOS (unlikely but possible). So I removed too much in this cleanup. > I'll send another patch set that puts this back.
I discovered the problem... In a rearrangement of the patches this change does happen but in a later patch [5/8]:
/* Initialize UV hubless systems */ static __init int uv_system_init_hubless(void) { @@ -1468,6 +1555,10 @@ static __init int uv_system_init_hubless /* Init kernel/BIOS interface */ rc = uv_bios_init();
+ /* Create user access node if UVsystab available */ + if (rc >= 0) + uv_setup_proc_files(1); + return rc; }
The mistake you saw [in patch 3/8] is very short lived... Hopefully no need for another patch set? > > Thanks, > Mike > >> >> Am I the only one who immediately sees the trivial C transformation >> through which this function could lose a local variable and become 4 >> lines shorter? >> >> And this function got two Reviewed-by tags... >> >> Thanks, >> >> Ingo >>
| |