diff mbox series

[1/1] um: fix error return code in winch_tramp()

Message ID 20210508032239.2177-1-thunder.leizhen@huawei.com
State Accepted
Headers show
Series [1/1] um: fix error return code in winch_tramp() | expand

Commit Message

Leizhen (ThunderTown) May 8, 2021, 3:22 a.m. UTC
Fix to return a negative error code from the error handling case instead
of 0, as done elsewhere in this function.

Fixes: 89df6bfc0405 ("uml: DEBUG_SHIRQ fixes")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 arch/um/drivers/chan_user.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Anton Ivanov May 8, 2021, 9:13 a.m. UTC | #1
On 08/05/2021 04:22, Zhen Lei wrote:
> Fix to return a negative error code from the error handling case instead
> of 0, as done elsewhere in this function.
> 
> Fixes: 89df6bfc0405 ("uml: DEBUG_SHIRQ fixes")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>   arch/um/drivers/chan_user.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
> index d8845d4aac6a..6040817c036f 100644
> --- a/arch/um/drivers/chan_user.c
> +++ b/arch/um/drivers/chan_user.c
> @@ -256,7 +256,8 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
>   		goto out_close;
>   	}
>   
> -	if (os_set_fd_block(*fd_out, 0)) {
> +	err = os_set_fd_block(*fd_out, 0);
> +	if (err) {
>   		printk(UM_KERN_ERR "winch_tramp: failed to set thread_fd "
>   		       "non-blocking.\n");
>   		goto out_close;
> 
Acked-By: anton.ivanov@cambridgegreys.com
Nathan Chancellor March 23, 2022, 7:41 p.m. UTC | #2
Hello,

On Sat, May 08, 2021 at 11:22:39AM +0800, Zhen Lei wrote:
> Fix to return a negative error code from the error handling case instead
> of 0, as done elsewhere in this function.
> 
> Fixes: 89df6bfc0405 ("uml: DEBUG_SHIRQ fixes")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  arch/um/drivers/chan_user.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
> index d8845d4aac6a..6040817c036f 100644
> --- a/arch/um/drivers/chan_user.c
> +++ b/arch/um/drivers/chan_user.c
> @@ -256,7 +256,8 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
>  		goto out_close;
>  	}
>  
> -	if (os_set_fd_block(*fd_out, 0)) {
> +	err = os_set_fd_block(*fd_out, 0);
> +	if (err) {
>  		printk(UM_KERN_ERR "winch_tramp: failed to set thread_fd "
>  		       "non-blocking.\n");
>  		goto out_close;
> -- 
> 2.25.1
> 
> 
> 

Sorry for the necro bump but this patch as commit ccf1236ecac4 ("um: fix
error return code in winch_tramp()") prevents UML from exiting cleanly
when it is called from within in a shell script. It is still
reproducible at next-20220323. I did see a patch from Richard that
touches this area but that patch does not make a difference:

https://lore.kernel.org/r/20220101215810.13260-3-richard@nod.at/

My bisect log:

# bad: [7d2a07b769330c34b4deabeed939325c77a7ec2f] Linux 5.14
# good: [62fb9874f5da54fdb243003b386128037319b219] Linux 5.13
git bisect start 'v5.14' 'v5.13'
# good: [406254918b232db198ed60f5bf1f8b84d96bca00] Merge tag 'perf-tools-for-v5.14-2021-07-01' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
git bisect good 406254918b232db198ed60f5bf1f8b84d96bca00
# good: [4ea90317956718e0648e1f87e56530db809a5a04] Merge tag 'for-linus-5.14-rc1-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip
git bisect good 4ea90317956718e0648e1f87e56530db809a5a04
# bad: [65ca89c2b12cca0d473f3dd54267568ad3af55cc] ASoC: intel: atom: Fix breakage for PCM buffer address setup
git bisect bad 65ca89c2b12cca0d473f3dd54267568ad3af55cc
# bad: [6e207b882159ed3e35a4cd4ff0fc155cce5e3cbc] Merge tag 'arm-soc-5.14' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc
git bisect bad 6e207b882159ed3e35a4cd4ff0fc155cce5e3cbc
# good: [f55966571d5eb2876a11e48e798b4592fa1ffbb7] Merge tag 'drm-next-2021-07-08-1' of git://anongit.freedesktop.org/drm/drm
git bisect good f55966571d5eb2876a11e48e798b4592fa1ffbb7
# bad: [1459718d7d79013a4814275c466e0b32da6a26bc] Merge tag 'powerpc-5.14-2' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
git bisect bad 1459718d7d79013a4814275c466e0b32da6a26bc
# good: [227c4d507c71acb7bece298a98d83e5b44433f62] Merge tag 'f2fs-for-5.14-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs
git bisect good 227c4d507c71acb7bece298a98d83e5b44433f62
# good: [96890bc2eaa1f6bfc1b194e0f0815a10824352a4] Merge tag 'nfs-for-5.14-1' of git://git.linux-nfs.org/projects/trondmy/linux-nfs
git bisect good 96890bc2eaa1f6bfc1b194e0f0815a10824352a4
# good: [e49d68ce7cc5a865ce14c1e57938438ab01c3ce3] Merge tag 'ext4_for_linus_stable' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4
git bisect good e49d68ce7cc5a865ce14c1e57938438ab01c3ce3
# bad: [ccf1236ecac476d9d2704866d9a476c86e387971] um: fix error return code in winch_tramp()
git bisect bad ccf1236ecac476d9d2704866d9a476c86e387971
# good: [68f5d3f3b6543266b29e047cfaf9842333019b4c] um: add PCI over virtio emulation driver
git bisect good 68f5d3f3b6543266b29e047cfaf9842333019b4c
# good: [c0ecca6604b80e438b032578634c6e133c7028f6] um: enable the use of optimized xor routines in UML
git bisect good c0ecca6604b80e438b032578634c6e133c7028f6
# good: [80f849bf541ef9b633a9c08ac208f9c9afd14eb9] um: implement flush_cache_vmap/flush_cache_vunmap
git bisect good 80f849bf541ef9b633a9c08ac208f9c9afd14eb9
# good: [b77e81fbe5f5fb4ad9a61ec80f6d1e30b6da093a] um: fix error return code in slip_open()
git bisect good b77e81fbe5f5fb4ad9a61ec80f6d1e30b6da093a
# first bad commit: [ccf1236ecac476d9d2704866d9a476c86e387971] um: fix error return code in winch_tramp()

$ make -skj"$(nproc)" ARCH=um mrproper defconfig all

$ ./linux ubd0=...
...
Run /sbin/init as init process
EXT4-fs (ubda): re-mounted. Quota mode: none.
Starting syslogd: OK
Starting klogd: OK
Running sysctl: OK
Initializing random number generator: OK
Saving random seed: OK
Starting network: OK
Linux version 5.17.0-next-20220323 (nathan@dev-arch.thelio-3990X) (gcc (GCC) 11.2.0, GNU ld (GNU Binutils) 2.38) #1 Wed Mar 23 12:05:22 MST 2022
Stopping network: OK
Saving random seed: OK
Stopping klogd: OK
Stopping syslogd: OK
EXT4-fs (ubda): re-mounted. Quota mode: none.
The system is going down NOW!
Sent SIGTERM to all processes
Sent SIGKILL to all processes
Requesting system poweroff
reboot: System halted

$ echo $?
0

$ cat test.sh
#!/usr/bin/env bash

./linux ubd0=...

$ ./test.sh
...
Run /sbin/init as init process
EXT4-fs (ubda): re-mounted. Quota mode: none.
Starting syslogd: OK
Starting klogd: OK
Running sysctl: OK
Initializing random number generator: OK
Saving random seed: OK
Starting network: OK
Linux version 5.17.0-next-20220323 (nathan@dev-arch.thelio-3990X) (gcc (GCC) 11.2.0, GNU ld (GNU Binutils) 2.38) #1 Wed Mar 23 12:10:31 MST 2022
Stopping network: OK
Saving random seed: OK
Stopping klogd: OK
Stopping syslogd: OK
EXT4-fs (ubda): re-mounted. Quota mode: none.
The system is going down NOW!
Sent SIGTERM to all processes
Sent SIGKILL to all processes
Requesting system poweroff
reboot: System halted
./test.sh: line 5: 970978 Killed                  ./linux ubd0=...

$ echo $?
137

The rootfs is a simple Buildroot image, which just prints the version
string then runs "poweroff". It is available at:

https://github.com/nathanchance/boot-utils/raw/bd4b962ee12e00f666eef12e3413a79d334a0685/images/x86_64/rootfs.ext4.zst

in case it helps. I am happy to provide more information or test patches
as necessary.

Cheers,
Nathan
Thorsten Leemhuis March 24, 2022, 12:40 p.m. UTC | #3
[TLDR: I'm adding the regression report below to regzbot, the Linux
kernel regression tracking bot; all text you find below is compiled from
a few templates paragraphs you might have encountered already already
from similar mails.]

Hi, this is your Linux kernel regression tracker. Top-posting for once,
to make this easily accessible to everyone.

Thanks for the report.

To be sure below issue doesn't fall through the cracks unnoticed, I'm
adding it to regzbot, my Linux kernel regression tracking bot:

#regzbot ^introduced ccf1236ecac4
#regzbot title uml: UML fails to exit cleanly when it is called from
within in a shell script
#regzbot ignore-activity

If it turns out this isn't a regression, free free to remove it from the
tracking by sending a reply to this thread containing a paragraph like
"#regzbot invalid: reason why this is invalid" (without the quotes).

Reminder for developers: when fixing the issue, please add a 'Link:'
tags pointing to the report (the mail) using lore.kernel.org/r/, as
explained in 'Documentation/process/submitting-patches.rst' and
'Documentation/process/5.Posting.rst'. Regzbot needs them to
automatically connect reports with fixes, but they are useful in
general, too.

I'm sending this to everyone that got the initial report, to make
everyone aware of the tracking. I also hope that messages like this
motivate people to directly get at least the regression mailing list and
ideally even regzbot involved when dealing with regressions, as messages
like this wouldn't be needed then. And don't worry, if I need to send
other mails regarding this regression only relevant for regzbot I'll
send them to the regressions lists only (with a tag in the subject so
people can filter them away). With a bit of luck no such messages will
be needed anyway.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I'm getting a lot of
reports on my table. I can only look briefly into most of them and lack
knowledge about most of the areas they concern. I thus unfortunately
will sometimes get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
in a public reply, it's in everyone's interest to set the public record
straight.


On 23.03.22 20:41, Nathan Chancellor wrote:
> Hello,
> 
> On Sat, May 08, 2021 at 11:22:39AM +0800, Zhen Lei wrote:
>> Fix to return a negative error code from the error handling case instead
>> of 0, as done elsewhere in this function.
>>
>> Fixes: 89df6bfc0405 ("uml: DEBUG_SHIRQ fixes")
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  arch/um/drivers/chan_user.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
>> index d8845d4aac6a..6040817c036f 100644
>> --- a/arch/um/drivers/chan_user.c
>> +++ b/arch/um/drivers/chan_user.c
>> @@ -256,7 +256,8 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
>>  		goto out_close;
>>  	}
>>  
>> -	if (os_set_fd_block(*fd_out, 0)) {
>> +	err = os_set_fd_block(*fd_out, 0);
>> +	if (err) {
>>  		printk(UM_KERN_ERR "winch_tramp: failed to set thread_fd "
>>  		       "non-blocking.\n");
>>  		goto out_close;
>> -- 
>> 2.25.1
>>
>>
>>
> 
> Sorry for the necro bump but this patch as commit ccf1236ecac4 ("um: fix
> error return code in winch_tramp()") prevents UML from exiting cleanly
> when it is called from within in a shell script. It is still
> reproducible at next-20220323. I did see a patch from Richard that
> touches this area but that patch does not make a difference:
> 
> https://lore.kernel.org/r/20220101215810.13260-3-richard@nod.at/
> 
> My bisect log:
> 
> # bad: [7d2a07b769330c34b4deabeed939325c77a7ec2f] Linux 5.14
> # good: [62fb9874f5da54fdb243003b386128037319b219] Linux 5.13
> git bisect start 'v5.14' 'v5.13'
> # good: [406254918b232db198ed60f5bf1f8b84d96bca00] Merge tag 'perf-tools-for-v5.14-2021-07-01' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
> git bisect good 406254918b232db198ed60f5bf1f8b84d96bca00
> # good: [4ea90317956718e0648e1f87e56530db809a5a04] Merge tag 'for-linus-5.14-rc1-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip
> git bisect good 4ea90317956718e0648e1f87e56530db809a5a04
> # bad: [65ca89c2b12cca0d473f3dd54267568ad3af55cc] ASoC: intel: atom: Fix breakage for PCM buffer address setup
> git bisect bad 65ca89c2b12cca0d473f3dd54267568ad3af55cc
> # bad: [6e207b882159ed3e35a4cd4ff0fc155cce5e3cbc] Merge tag 'arm-soc-5.14' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc
> git bisect bad 6e207b882159ed3e35a4cd4ff0fc155cce5e3cbc
> # good: [f55966571d5eb2876a11e48e798b4592fa1ffbb7] Merge tag 'drm-next-2021-07-08-1' of git://anongit.freedesktop.org/drm/drm
> git bisect good f55966571d5eb2876a11e48e798b4592fa1ffbb7
> # bad: [1459718d7d79013a4814275c466e0b32da6a26bc] Merge tag 'powerpc-5.14-2' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> git bisect bad 1459718d7d79013a4814275c466e0b32da6a26bc
> # good: [227c4d507c71acb7bece298a98d83e5b44433f62] Merge tag 'f2fs-for-5.14-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs
> git bisect good 227c4d507c71acb7bece298a98d83e5b44433f62
> # good: [96890bc2eaa1f6bfc1b194e0f0815a10824352a4] Merge tag 'nfs-for-5.14-1' of git://git.linux-nfs.org/projects/trondmy/linux-nfs
> git bisect good 96890bc2eaa1f6bfc1b194e0f0815a10824352a4
> # good: [e49d68ce7cc5a865ce14c1e57938438ab01c3ce3] Merge tag 'ext4_for_linus_stable' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4
> git bisect good e49d68ce7cc5a865ce14c1e57938438ab01c3ce3
> # bad: [ccf1236ecac476d9d2704866d9a476c86e387971] um: fix error return code in winch_tramp()
> git bisect bad ccf1236ecac476d9d2704866d9a476c86e387971
> # good: [68f5d3f3b6543266b29e047cfaf9842333019b4c] um: add PCI over virtio emulation driver
> git bisect good 68f5d3f3b6543266b29e047cfaf9842333019b4c
> # good: [c0ecca6604b80e438b032578634c6e133c7028f6] um: enable the use of optimized xor routines in UML
> git bisect good c0ecca6604b80e438b032578634c6e133c7028f6
> # good: [80f849bf541ef9b633a9c08ac208f9c9afd14eb9] um: implement flush_cache_vmap/flush_cache_vunmap
> git bisect good 80f849bf541ef9b633a9c08ac208f9c9afd14eb9
> # good: [b77e81fbe5f5fb4ad9a61ec80f6d1e30b6da093a] um: fix error return code in slip_open()
> git bisect good b77e81fbe5f5fb4ad9a61ec80f6d1e30b6da093a
> # first bad commit: [ccf1236ecac476d9d2704866d9a476c86e387971] um: fix error return code in winch_tramp()
> 
> $ make -skj"$(nproc)" ARCH=um mrproper defconfig all
> 
> $ ./linux ubd0=...
> ...
> Run /sbin/init as init process
> EXT4-fs (ubda): re-mounted. Quota mode: none.
> Starting syslogd: OK
> Starting klogd: OK
> Running sysctl: OK
> Initializing random number generator: OK
> Saving random seed: OK
> Starting network: OK
> Linux version 5.17.0-next-20220323 (nathan@dev-arch.thelio-3990X) (gcc (GCC) 11.2.0, GNU ld (GNU Binutils) 2.38) #1 Wed Mar 23 12:05:22 MST 2022
> Stopping network: OK
> Saving random seed: OK
> Stopping klogd: OK
> Stopping syslogd: OK
> EXT4-fs (ubda): re-mounted. Quota mode: none.
> The system is going down NOW!
> Sent SIGTERM to all processes
> Sent SIGKILL to all processes
> Requesting system poweroff
> reboot: System halted
> 
> $ echo $?
> 0
> 
> $ cat test.sh
> #!/usr/bin/env bash
> 
> ./linux ubd0=...
> 
> $ ./test.sh
> ...
> Run /sbin/init as init process
> EXT4-fs (ubda): re-mounted. Quota mode: none.
> Starting syslogd: OK
> Starting klogd: OK
> Running sysctl: OK
> Initializing random number generator: OK
> Saving random seed: OK
> Starting network: OK
> Linux version 5.17.0-next-20220323 (nathan@dev-arch.thelio-3990X) (gcc (GCC) 11.2.0, GNU ld (GNU Binutils) 2.38) #1 Wed Mar 23 12:10:31 MST 2022
> Stopping network: OK
> Saving random seed: OK
> Stopping klogd: OK
> Stopping syslogd: OK
> EXT4-fs (ubda): re-mounted. Quota mode: none.
> The system is going down NOW!
> Sent SIGTERM to all processes
> Sent SIGKILL to all processes
> Requesting system poweroff
> reboot: System halted
> ./test.sh: line 5: 970978 Killed                  ./linux ubd0=...
> 
> $ echo $?
> 137
> 
> The rootfs is a simple Buildroot image, which just prints the version
> string then runs "poweroff". It is available at:
> 
> https://github.com/nathanchance/boot-utils/raw/bd4b962ee12e00f666eef12e3413a79d334a0685/images/x86_64/rootfs.ext4.zst
> 
> in case it helps. I am happy to provide more information or test patches
> as necessary.
> 
> Cheers,
> Nathan
> 
>
Thorsten Leemhuis April 10, 2022, 8:41 a.m. UTC | #4
Hi, this is your Linux kernel regression tracker. Top-posting for once,
to make this easily accessible to everyone.

Zhen Lei, Richard, what's up here? Below regression report is more than
two weeks old now and afaics didn't even get a single reply.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I'm getting a lot of
reports on my table. I can only look briefly into most of them and lack
knowledge about most of the areas they concern. I thus unfortunately
will sometimes get things wrong or miss something important. I hope
that's not the case here; if you think it is, don't hesitate to tell me
in a public reply, it's in everyone's interest to set the public record
straight.



On 23.03.22 20:41, Nathan Chancellor wrote:
> Hello,
> 
> On Sat, May 08, 2021 at 11:22:39AM +0800, Zhen Lei wrote:
>> Fix to return a negative error code from the error handling case instead
>> of 0, as done elsewhere in this function.
>>
>> Fixes: 89df6bfc0405 ("uml: DEBUG_SHIRQ fixes")
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>>  arch/um/drivers/chan_user.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
>> index d8845d4aac6a..6040817c036f 100644
>> --- a/arch/um/drivers/chan_user.c
>> +++ b/arch/um/drivers/chan_user.c
>> @@ -256,7 +256,8 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
>>  		goto out_close;
>>  	}
>>  
>> -	if (os_set_fd_block(*fd_out, 0)) {
>> +	err = os_set_fd_block(*fd_out, 0);
>> +	if (err) {
>>  		printk(UM_KERN_ERR "winch_tramp: failed to set thread_fd "
>>  		       "non-blocking.\n");
>>  		goto out_close;
>> -- 
>> 2.25.1
>>
>>
>>
> 
> Sorry for the necro bump but this patch as commit ccf1236ecac4 ("um: fix
> error return code in winch_tramp()") prevents UML from exiting cleanly
> when it is called from within in a shell script. It is still
> reproducible at next-20220323. I did see a patch from Richard that
> touches this area but that patch does not make a difference:
> 
> https://lore.kernel.org/r/20220101215810.13260-3-richard@nod.at/
> 
> My bisect log:
> 
> # bad: [7d2a07b769330c34b4deabeed939325c77a7ec2f] Linux 5.14
> # good: [62fb9874f5da54fdb243003b386128037319b219] Linux 5.13
> git bisect start 'v5.14' 'v5.13'
> # good: [406254918b232db198ed60f5bf1f8b84d96bca00] Merge tag 'perf-tools-for-v5.14-2021-07-01' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux
> git bisect good 406254918b232db198ed60f5bf1f8b84d96bca00
> # good: [4ea90317956718e0648e1f87e56530db809a5a04] Merge tag 'for-linus-5.14-rc1-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip
> git bisect good 4ea90317956718e0648e1f87e56530db809a5a04
> # bad: [65ca89c2b12cca0d473f3dd54267568ad3af55cc] ASoC: intel: atom: Fix breakage for PCM buffer address setup
> git bisect bad 65ca89c2b12cca0d473f3dd54267568ad3af55cc
> # bad: [6e207b882159ed3e35a4cd4ff0fc155cce5e3cbc] Merge tag 'arm-soc-5.14' of git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc
> git bisect bad 6e207b882159ed3e35a4cd4ff0fc155cce5e3cbc
> # good: [f55966571d5eb2876a11e48e798b4592fa1ffbb7] Merge tag 'drm-next-2021-07-08-1' of git://anongit.freedesktop.org/drm/drm
> git bisect good f55966571d5eb2876a11e48e798b4592fa1ffbb7
> # bad: [1459718d7d79013a4814275c466e0b32da6a26bc] Merge tag 'powerpc-5.14-2' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux
> git bisect bad 1459718d7d79013a4814275c466e0b32da6a26bc
> # good: [227c4d507c71acb7bece298a98d83e5b44433f62] Merge tag 'f2fs-for-5.14-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs
> git bisect good 227c4d507c71acb7bece298a98d83e5b44433f62
> # good: [96890bc2eaa1f6bfc1b194e0f0815a10824352a4] Merge tag 'nfs-for-5.14-1' of git://git.linux-nfs.org/projects/trondmy/linux-nfs
> git bisect good 96890bc2eaa1f6bfc1b194e0f0815a10824352a4
> # good: [e49d68ce7cc5a865ce14c1e57938438ab01c3ce3] Merge tag 'ext4_for_linus_stable' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4
> git bisect good e49d68ce7cc5a865ce14c1e57938438ab01c3ce3
> # bad: [ccf1236ecac476d9d2704866d9a476c86e387971] um: fix error return code in winch_tramp()
> git bisect bad ccf1236ecac476d9d2704866d9a476c86e387971
> # good: [68f5d3f3b6543266b29e047cfaf9842333019b4c] um: add PCI over virtio emulation driver
> git bisect good 68f5d3f3b6543266b29e047cfaf9842333019b4c
> # good: [c0ecca6604b80e438b032578634c6e133c7028f6] um: enable the use of optimized xor routines in UML
> git bisect good c0ecca6604b80e438b032578634c6e133c7028f6
> # good: [80f849bf541ef9b633a9c08ac208f9c9afd14eb9] um: implement flush_cache_vmap/flush_cache_vunmap
> git bisect good 80f849bf541ef9b633a9c08ac208f9c9afd14eb9
> # good: [b77e81fbe5f5fb4ad9a61ec80f6d1e30b6da093a] um: fix error return code in slip_open()
> git bisect good b77e81fbe5f5fb4ad9a61ec80f6d1e30b6da093a
> # first bad commit: [ccf1236ecac476d9d2704866d9a476c86e387971] um: fix error return code in winch_tramp()
> 
> $ make -skj"$(nproc)" ARCH=um mrproper defconfig all
> 
> $ ./linux ubd0=...
> ...
> Run /sbin/init as init process
> EXT4-fs (ubda): re-mounted. Quota mode: none.
> Starting syslogd: OK
> Starting klogd: OK
> Running sysctl: OK
> Initializing random number generator: OK
> Saving random seed: OK
> Starting network: OK
> Linux version 5.17.0-next-20220323 (nathan@dev-arch.thelio-3990X) (gcc (GCC) 11.2.0, GNU ld (GNU Binutils) 2.38) #1 Wed Mar 23 12:05:22 MST 2022
> Stopping network: OK
> Saving random seed: OK
> Stopping klogd: OK
> Stopping syslogd: OK
> EXT4-fs (ubda): re-mounted. Quota mode: none.
> The system is going down NOW!
> Sent SIGTERM to all processes
> Sent SIGKILL to all processes
> Requesting system poweroff
> reboot: System halted
> 
> $ echo $?
> 0
> 
> $ cat test.sh
> #!/usr/bin/env bash
> 
> ./linux ubd0=...
> 
> $ ./test.sh
> ...
> Run /sbin/init as init process
> EXT4-fs (ubda): re-mounted. Quota mode: none.
> Starting syslogd: OK
> Starting klogd: OK
> Running sysctl: OK
> Initializing random number generator: OK
> Saving random seed: OK
> Starting network: OK
> Linux version 5.17.0-next-20220323 (nathan@dev-arch.thelio-3990X) (gcc (GCC) 11.2.0, GNU ld (GNU Binutils) 2.38) #1 Wed Mar 23 12:10:31 MST 2022
> Stopping network: OK
> Saving random seed: OK
> Stopping klogd: OK
> Stopping syslogd: OK
> EXT4-fs (ubda): re-mounted. Quota mode: none.
> The system is going down NOW!
> Sent SIGTERM to all processes
> Sent SIGKILL to all processes
> Requesting system poweroff
> reboot: System halted
> ./test.sh: line 5: 970978 Killed                  ./linux ubd0=...
> 
> $ echo $?
> 137
> 
> The rootfs is a simple Buildroot image, which just prints the version
> string then runs "poweroff". It is available at:
> 
> https://github.com/nathanchance/boot-utils/raw/bd4b962ee12e00f666eef12e3413a79d334a0685/images/x86_64/rootfs.ext4.zst
> 
> in case it helps. I am happy to provide more information or test patches
> as necessary.
> 
> Cheers,
> Nathan
> 
>
Richard Weinberger April 10, 2022, 9:33 a.m. UTC | #5
----- Ursprüngliche Mail -----
> Von: "Thorsten Leemhuis" <regressions@leemhuis.info>
> Hi, this is your Linux kernel regression tracker. Top-posting for once,
> to make this easily accessible to everyone.
> 
> Zhen Lei, Richard, what's up here? Below regression report is more than
> two weeks old now and afaics didn't even get a single reply.

Sorry, but UML is leisure pursuit, so I can only work on it when my
schedule permits it.
 
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> 
> P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> reports on my table. I can only look briefly into most of them and lack
> knowledge about most of the areas they concern. I thus unfortunately
> will sometimes get things wrong or miss something important. I hope
> that's not the case here; if you think it is, don't hesitate to tell me
> in a public reply, it's in everyone's interest to set the public record
> straight.
> 
> 
> 
> On 23.03.22 20:41, Nathan Chancellor wrote:

[...]

>> in case it helps. I am happy to provide more information or test patches
>> as necessary.

Nathan, can you provide me the error code from os_set_fd_block()?
My best guess is that setting the fd is optional here and UML does
not expect failure here.

Thanks,
//richard
Nathan Chancellor April 11, 2022, 5:48 p.m. UTC | #6
Hi Richard,

On Sun, Apr 10, 2022 at 11:33:33AM +0200, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Thorsten Leemhuis" <regressions@leemhuis.info>
> > Hi, this is your Linux kernel regression tracker. Top-posting for once,
> > to make this easily accessible to everyone.
> > 
> > Zhen Lei, Richard, what's up here? Below regression report is more than
> > two weeks old now and afaics didn't even get a single reply.
> 
> Sorry, but UML is leisure pursuit, so I can only work on it when my
> schedule permits it.

No worries, it is easy enough to work around for our use case.

> > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> > 
> > P.S.: As the Linux kernel's regression tracker I'm getting a lot of
> > reports on my table. I can only look briefly into most of them and lack
> > knowledge about most of the areas they concern. I thus unfortunately
> > will sometimes get things wrong or miss something important. I hope
> > that's not the case here; if you think it is, don't hesitate to tell me
> > in a public reply, it's in everyone's interest to set the public record
> > straight.
> > 
> > 
> > 
> > On 23.03.22 20:41, Nathan Chancellor wrote:
> 
> [...]
> 
> >> in case it helps. I am happy to provide more information or test patches
> >> as necessary.
> 
> Nathan, can you provide me the error code from os_set_fd_block()?
> My best guess is that setting the fd is optional here and UML does
> not expect failure here.

I attempted to print out the error code but it seems like there is no
output in the console after "reboot: System halted". If I add an
unconditional print right before the call to os_set_fd_block(), I see it
during start up but I do not see it during shutdown. Is there some way
to see that console output during shutdown?

Cheers,
Nathan
Richard Weinberger April 11, 2022, 6:45 p.m. UTC | #7
----- Ursprüngliche Mail -----
> Von: "Nathan Chancellor" <nathan@kernel.org>
> I attempted to print out the error code but it seems like there is no
> output in the console after "reboot: System halted". If I add an
> unconditional print right before the call to os_set_fd_block(), I see it
> during start up but I do not see it during shutdown. Is there some way
> to see that console output during shutdown?

I think in this case the easiest way is attaching gdb with a breakpoint.

Thanks,
//richard
Thorsten Leemhuis May 20, 2022, 6:08 a.m. UTC | #8
On 11.04.22 20:45, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "Nathan Chancellor" <nathan@kernel.org>
>> I attempted to print out the error code but it seems like there is no
>> output in the console after "reboot: System halted". If I add an
>> unconditional print right before the call to os_set_fd_block(), I see it
>> during start up but I do not see it during shutdown. Is there some way
>> to see that console output during shutdown?
> 
> I think in this case the easiest way is attaching gdb with a breakpoint.

I noticed this in my list of open regressions. It seems there wasn't any
progress to get this regression fixed (please let me know in case I
missed something), but I guess nobody considered it urgent which is
likely not that much of a problem in this case. Hence I'm moving this to
the back-burner:

#regzbot backburner: special case and UML a leisure pursuit for the
maintainer

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)

P.S.: As the Linux kernel's regression tracker I deal with a lot of
reports and sometimes miss something important when writing mails like
this. If that's the case here, don't hesitate to tell me in a public
reply, it's in everyone's interest to set the public record straight.
Nathan Chancellor May 20, 2022, 3:55 p.m. UTC | #9
On Fri, May 20, 2022 at 08:08:01AM +0200, Thorsten Leemhuis wrote:
> On 11.04.22 20:45, Richard Weinberger wrote:
> > ----- Ursprüngliche Mail -----
> >> Von: "Nathan Chancellor" <nathan@kernel.org>
> >> I attempted to print out the error code but it seems like there is no
> >> output in the console after "reboot: System halted". If I add an
> >> unconditional print right before the call to os_set_fd_block(), I see it
> >> during start up but I do not see it during shutdown. Is there some way
> >> to see that console output during shutdown?
> > 
> > I think in this case the easiest way is attaching gdb with a breakpoint.
> 
> I noticed this in my list of open regressions. It seems there wasn't any
> progress to get this regression fixed (please let me know in case I
> missed something), but I guess nobody considered it urgent which is
> likely not that much of a problem in this case.

Yes, sorry, I tried to get gdb to reveal something but I couldn't get it
to work then I had to move onto other work. We have worked around this
for the time being but it would still be nice to figure out what is
going on here; I am just not sure when I am going to have time to
participate in that process.

Cheers,
Nathan
Johannes Berg May 20, 2022, 5:18 p.m. UTC | #10
On Fri, 2022-05-20 at 08:55 -0700, Nathan Chancellor wrote:
> On Fri, May 20, 2022 at 08:08:01AM +0200, Thorsten Leemhuis wrote:
> > On 11.04.22 20:45, Richard Weinberger wrote:
> > > ----- Ursprüngliche Mail -----
> > > > Von: "Nathan Chancellor" <nathan@kernel.org>
> > > > I attempted to print out the error code but it seems like there is no
> > > > output in the console after "reboot: System halted". If I add an
> > > > unconditional print right before the call to os_set_fd_block(), I see it
> > > > during start up but I do not see it during shutdown. Is there some way
> > > > to see that console output during shutdown?
> > > 
> > > I think in this case the easiest way is attaching gdb with a breakpoint.
> > 
> > I noticed this in my list of open regressions. It seems there wasn't any
> > progress to get this regression fixed (please let me know in case I
> > missed something), but I guess nobody considered it urgent which is
> > likely not that much of a problem in this case.
> 
> Yes, sorry, I tried to get gdb to reveal something but I couldn't get it
> to work then I had to move onto other work. We have worked around this
> for the time being but it would still be nice to figure out what is
> going on here; I am just not sure when I am going to have time to
> participate in that process.
> 

This fixes it for me, can you check it?

diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
index 6040817c036f..25727ed648b7 100644
--- a/arch/um/drivers/chan_user.c
+++ b/arch/um/drivers/chan_user.c
@@ -220,7 +220,7 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
 		       unsigned long *stack_out)
 {
 	struct winch_data data;
-	int fds[2], n, err;
+	int fds[2], n, err, pid;
 	char c;
 
 	err = os_pipe(fds, 1, 1);
@@ -238,8 +238,9 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
 	 * problem with /dev/net/tun, which if held open by this
 	 * thread, prevents the TUN/TAP device from being reused.
 	 */
-	err = run_helper_thread(winch_thread, &data, CLONE_FILES, stack_out);
-	if (err < 0) {
+	pid = run_helper_thread(winch_thread, &data, CLONE_FILES, stack_out);
+	if (pid < 0) {
+		err = pid;
 		printk(UM_KERN_ERR "fork of winch_thread failed - errno = %d\n",
 		       -err);
 		goto out_close;
@@ -263,7 +264,7 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
 		goto out_close;
 	}
 
-	return err;
+	return pid;
 
  out_close:
 	close(fds[1]);


Kind of obvious, really. :)

johannes
Nathan Chancellor May 20, 2022, 5:31 p.m. UTC | #11
On Fri, May 20, 2022 at 07:18:28PM +0200, Johannes Berg wrote:
> On Fri, 2022-05-20 at 08:55 -0700, Nathan Chancellor wrote:
> > On Fri, May 20, 2022 at 08:08:01AM +0200, Thorsten Leemhuis wrote:
> > > On 11.04.22 20:45, Richard Weinberger wrote:
> > > > ----- Ursprüngliche Mail -----
> > > > > Von: "Nathan Chancellor" <nathan@kernel.org>
> > > > > I attempted to print out the error code but it seems like there is no
> > > > > output in the console after "reboot: System halted". If I add an
> > > > > unconditional print right before the call to os_set_fd_block(), I see it
> > > > > during start up but I do not see it during shutdown. Is there some way
> > > > > to see that console output during shutdown?
> > > > 
> > > > I think in this case the easiest way is attaching gdb with a breakpoint.
> > > 
> > > I noticed this in my list of open regressions. It seems there wasn't any
> > > progress to get this regression fixed (please let me know in case I
> > > missed something), but I guess nobody considered it urgent which is
> > > likely not that much of a problem in this case.
> > 
> > Yes, sorry, I tried to get gdb to reveal something but I couldn't get it
> > to work then I had to move onto other work. We have worked around this
> > for the time being but it would still be nice to figure out what is
> > going on here; I am just not sure when I am going to have time to
> > participate in that process.
> > 
> 
> This fixes it for me, can you check it?

Yes, that works for me as well, thanks for looking into it!

Tested-by: Nathan Chancellor <nathan@kernel.org>

> diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
> index 6040817c036f..25727ed648b7 100644
> --- a/arch/um/drivers/chan_user.c
> +++ b/arch/um/drivers/chan_user.c
> @@ -220,7 +220,7 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
>  		       unsigned long *stack_out)
>  {
>  	struct winch_data data;
> -	int fds[2], n, err;
> +	int fds[2], n, err, pid;
>  	char c;
>  
>  	err = os_pipe(fds, 1, 1);
> @@ -238,8 +238,9 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
>  	 * problem with /dev/net/tun, which if held open by this
>  	 * thread, prevents the TUN/TAP device from being reused.
>  	 */
> -	err = run_helper_thread(winch_thread, &data, CLONE_FILES, stack_out);
> -	if (err < 0) {
> +	pid = run_helper_thread(winch_thread, &data, CLONE_FILES, stack_out);
> +	if (pid < 0) {
> +		err = pid;
>  		printk(UM_KERN_ERR "fork of winch_thread failed - errno = %d\n",
>  		       -err);
>  		goto out_close;
> @@ -263,7 +264,7 @@ static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
>  		goto out_close;
>  	}
>  
> -	return err;
> +	return pid;
>  
>   out_close:
>  	close(fds[1]);
> 
> 
> Kind of obvious, really. :)
> 
> johannes
diff mbox series

Patch

diff --git a/arch/um/drivers/chan_user.c b/arch/um/drivers/chan_user.c
index d8845d4aac6a..6040817c036f 100644
--- a/arch/um/drivers/chan_user.c
+++ b/arch/um/drivers/chan_user.c
@@ -256,7 +256,8 @@  static int winch_tramp(int fd, struct tty_port *port, int *fd_out,
 		goto out_close;
 	}
 
-	if (os_set_fd_block(*fd_out, 0)) {
+	err = os_set_fd_block(*fd_out, 0);
+	if (err) {
 		printk(UM_KERN_ERR "winch_tramp: failed to set thread_fd "
 		       "non-blocking.\n");
 		goto out_close;