# \[PATCH 1/1\] powerpc/ps3: Fix error return code in ps3\_register\_devices() - Subsystems: LINUX FOR POWERPC (32-BIT AND 64-BIT), PS3 PLATFORM SUPPORT, THE REST - Maintainers: Madhavan Srinivasan, Geoff Levand, Linus Torvalds - List: linuxppc-dev - 4 messages, 3 authors - Page: https://ratatoskr.run/linuxppc-dev/2021/05/9522772/t ## \[PATCH 1/1\] powerpc/ps3: Fix error return code in ps3\_register\_devices() - From: Zhen Lei \ - Date: 2021-05-18 06:59:35 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522772.md ```text When call ps3_start_probe_thread() failed, further initialization should be stopped and the returned error code should be propagated. Reported-by: Hulk Robot Signed-off-by: Zhen Lei --- arch/powerpc/platforms/ps3/device-init.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c index e87360a0fb40..9b6d8ca8fc01 100644 --- a/arch/powerpc/platforms/ps3/device-init.c +++ b/arch/powerpc/platforms/ps3/device-init.c @@ -955,6 +955,8 @@ static int __init ps3_register_devices(void) /* ps3_repository_dump_bus_info(); */ result = ps3_start_probe_thread(PS3_BUS_TYPE_STORAGE); + if (result < 0) + return result; ps3_register_vuart_devices(); -- 2.25.1 ``` ## Re: \[PATCH 1/1\] powerpc/ps3: Fix error return code in ps3\_register\_devices() - From: Michael Ellerman \ - Date: 2021-05-20 12:21:21 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522835.md ```text Zhen Lei [ref] writes: > When call ps3_start_probe_thread() failed, further initialization should > be stopped and the returned error code should be propagated. It's not clear to me that's a good change. > diff --git a/arch/powerpc/platforms/ps3/device-init.c b/arch/powerpc/platforms/ps3/device-init.c > index e87360a0fb40..9b6d8ca8fc01 100644 > --- a/arch/powerpc/platforms/ps3/device-init.c > +++ b/arch/powerpc/platforms/ps3/device-init.c > @@ -955,6 +955,8 @@ static int __init ps3_register_devices(void) > /* ps3_repository_dump_bus_info(); */ > > result = ps3_start_probe_thread(PS3_BUS_TYPE_STORAGE); > + if (result < 0) > + return result; If you bail out here you skip: > ps3_register_vuart_devices(); Which I suspect means there will be no console output? Presumably the system won't boot if the probe thread fails, but it might at least print an oops, whereas if we return we might get nothing at all. Though I'm just guessing, I don't know this code that well. Anyway please leave this code alone unless you're willing to test your changes, or at least provide a more thorough justification for them. cheers ``` ## Re: \[PATCH 1/1\] powerpc/ps3: Fix error return code in ps3\_register\_devices() - From: Geoff Levand \ - Date: 2021-05-23 20:17:00 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522871.md ```text Hi, On 5/20/21 5:20 AM, Michael Ellerman wrote: > Zhen Lei [ref] writes: >> When call ps3_start_probe_thread() failed, further initialization should >> be stopped and the returned error code should be propagated. ... >> --- a/arch/powerpc/platforms/ps3/device-init.c >> >> result = ps3_start_probe_thread(PS3_BUS_TYPE_STORAGE); >> + if (result < 0) >> + return result; > > If you bail out here you skip: > >> ps3_register_vuart_devices(); > > Which I suspect means there will be no console output? > > Presumably the system won't boot if the probe thread fails, but it might > at least print an oops, whereas if we return we might get nothing at > all. Though I'm just guessing, I don't know this code that well. That probe is for the storage devices (PS3_BUS_TYPE_STORAGE). There are cases where the system is usable even if the storage devices are not available, for example, when using an NFS root filesystem. ps3_start_probe_thread was made to be quite verbose on error to make up for it's return value not being checked. > Anyway please leave this code alone unless you're willing to test your > changes, or at least provide a more thorough justification for them. Agreed, this change should not be merged. -Geoff ``` ## Re: \[PATCH 1/1\] powerpc/ps3: Fix error return code in ps3\_register\_devices() - From: Leizhen (ThunderTown) \ - Date: 2021-05-24 01:40:51 - Message: https://ratatoskr.run/linuxppc-dev/2021/05/9522874.md ```text On 2021/5/24 4:15, Geoff Levand wrote: > Hi, > > On 5/20/21 5:20 AM, Michael Ellerman wrote: >> Zhen Lei [ref] writes: >>> When call ps3_start_probe_thread() failed, further initialization should >>> be stopped and the returned error code should be propagated. > ... >>> --- a/arch/powerpc/platforms/ps3/device-init.c >>> >>> result = ps3_start_probe_thread(PS3_BUS_TYPE_STORAGE); >>> + if (result < 0) >>> + return result; >> >> If you bail out here you skip: >> >>> ps3_register_vuart_devices(); >> >> Which I suspect means there will be no console output? >> >> Presumably the system won't boot if the probe thread fails, but it might >> at least print an oops, whereas if we return we might get nothing at >> all. Though I'm just guessing, I don't know this code that well. > > That probe is for the storage devices (PS3_BUS_TYPE_STORAGE). > > There are cases where the system is usable even if the storage > devices are not available, for example, when using an NFS root > filesystem. > > ps3_start_probe_thread was made to be quite verbose on error > to make up for it's return value not being checked. So should we delete the local variable 'result' and ignore the return value? - result = ps3_start_probe_thread(PS3_BUS_TYPE_STORAGE); - (void)ps3_start_probe_thread(PS3_BUS_TYPE_STORAGE); > >> Anyway please leave this code alone unless you're willing to test your >> changes, or at least provide a more thorough justification for them. > > Agreed, this change should not be merged. > > -Geoff > > . > ```