Re: [PATCH 1/1] powerpc/ps3: Fix error return code in ps3_register_devices()

From
Leizhen (ThunderTown) <hidden>
Date
2021-05-24 01:40:51
Reply to
Geoff Levand, 2021-05-23

On 2021/5/24 4:15, Geoff Levand wrote:
Hi,

On 5/20/21 5:20 AM, Michael Ellerman wrote:
quoted
Zhen Lei [off-list ref] writes:
quoted
When call ps3_start_probe_thread() failed, further initialization should
be stopped and the returned error code should be propagated.
...
quoted
quoted
--- 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:
quoted
 	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);
quoted
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

.
  
Thread (4 messages) 4 messages, 3 authors, last reply 2021-05-24
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help