* [PATCH v2 0/4] um: Make errno multi-thread safe
@ 2025-03-06 15:07 Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 1/4] um: Add pthread-based helper support Tiwei Bie
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Tiwei Bie @ 2025-03-06 15:07 UTC (permalink / raw)
To: richard, anton.ivanov, johannes; +Cc: linux-um, Tiwei Bie
v2:
- Do not define the thread handle type as void *;
- Minor tweaks to commit messages, log and comments;
v1:
https://lore.kernel.org/all/20250221084049.1332318-1-tiwei.btw@antgroup.com/
Tiwei Bie (4):
um: Add pthread-based helper support
um: ubd: Switch to the pthread-based helper
um: Switch to the pthread-based helper in sigio workaround
um: Prohibit the VM_CLONE flag in run_helper_thread()
arch/um/drivers/ubd.h | 6 ++--
arch/um/drivers/ubd_kern.c | 25 ++++++--------
arch/um/drivers/ubd_user.c | 14 ++++----
arch/um/include/shared/os.h | 5 +++
arch/um/os-Linux/helper.c | 67 +++++++++++++++++++++++++++++++++++++
arch/um/os-Linux/sigio.c | 44 +++++++++++-------------
6 files changed, 113 insertions(+), 48 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/4] um: Add pthread-based helper support
2025-03-06 15:07 [PATCH v2 0/4] um: Make errno multi-thread safe Tiwei Bie
@ 2025-03-06 15:07 ` Tiwei Bie
2025-03-18 13:06 ` Johannes Berg
2025-03-19 9:11 ` Johannes Berg
2025-03-06 15:07 ` [PATCH v2 2/4] um: ubd: Switch to the pthread-based helper Tiwei Bie
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Tiwei Bie @ 2025-03-06 15:07 UTC (permalink / raw)
To: richard, anton.ivanov, johannes; +Cc: linux-um, Tiwei Bie
Introduce a new set of utility functions that can be used to create
pthread-based helpers. Helper threads created in this way will ensure
thread safety for errno while sharing the same memory space.
Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
---
arch/um/include/shared/os.h | 5 +++
arch/um/os-Linux/helper.c | 63 +++++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h
index 5babad8c5f75..c4f8f990ffb8 100644
--- a/arch/um/include/shared/os.h
+++ b/arch/um/include/shared/os.h
@@ -225,6 +225,11 @@ extern int run_helper_thread(int (*proc)(void *), void *arg,
unsigned int flags, unsigned long *stack_out);
extern int helper_wait(int pid);
+struct os_helper_thread;
+int os_run_helper_thread(struct os_helper_thread **td_out,
+ void *(*routine)(void *), void *arg);
+void os_kill_helper_thread(struct os_helper_thread *td);
+void os_fix_helper_thread_signals(void);
/* umid.c */
extern int umid_file_name(char *name, char *buf, int len);
diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c
index 3cb8ac63be6e..5cb30773c511 100644
--- a/arch/um/os-Linux/helper.c
+++ b/arch/um/os-Linux/helper.c
@@ -8,6 +8,7 @@
#include <unistd.h>
#include <errno.h>
#include <sched.h>
+#include <pthread.h>
#include <linux/limits.h>
#include <sys/socket.h>
#include <sys/wait.h>
@@ -167,3 +168,65 @@ int helper_wait(int pid)
} else
return 0;
}
+
+struct os_helper_thread {
+ pthread_t handle;
+};
+
+int os_run_helper_thread(struct os_helper_thread **td_out,
+ void *(*routine)(void *), void *arg)
+{
+ struct os_helper_thread *td;
+ sigset_t sigset, oset;
+ int err, flags;
+
+ flags = __uml_cant_sleep() ? UM_GFP_ATOMIC : UM_GFP_KERNEL;
+ td = uml_kmalloc(sizeof(*td), flags);
+ if (!td)
+ return -ENOMEM;
+
+ sigfillset(&sigset);
+ if (sigprocmask(SIG_SETMASK, &sigset, &oset) < 0) {
+ err = -errno;
+ kfree(td);
+ return err;
+ }
+
+ err = pthread_create(&td->handle, NULL, routine, arg);
+
+ if (sigprocmask(SIG_SETMASK, &oset, NULL) < 0)
+ panic("Failed to restore the signal mask: %d", errno);
+
+ if (err != 0)
+ kfree(td);
+ else
+ *td_out = td;
+
+ return -err;
+}
+
+void os_kill_helper_thread(struct os_helper_thread *td)
+{
+ pthread_kill(td->handle, SIGKILL);
+ CATCH_EINTR(pthread_join(td->handle, NULL));
+ kfree(td);
+}
+
+void os_fix_helper_thread_signals(void)
+{
+ sigset_t sigset;
+
+ sigemptyset(&sigset);
+
+ sigaddset(&sigset, SIGWINCH);
+ sigaddset(&sigset, SIGPIPE);
+ sigaddset(&sigset, SIGPROF);
+ sigaddset(&sigset, SIGINT);
+ sigaddset(&sigset, SIGTERM);
+ sigaddset(&sigset, SIGCHLD);
+ sigaddset(&sigset, SIGALRM);
+ sigaddset(&sigset, SIGIO);
+ sigaddset(&sigset, SIGUSR1);
+
+ pthread_sigmask(SIG_SETMASK, &sigset, NULL);
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] um: ubd: Switch to the pthread-based helper
2025-03-06 15:07 [PATCH v2 0/4] um: Make errno multi-thread safe Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 1/4] um: Add pthread-based helper support Tiwei Bie
@ 2025-03-06 15:07 ` Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 3/4] um: Switch to the pthread-based helper in sigio workaround Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 4/4] um: Prohibit the VM_CLONE flag in run_helper_thread() Tiwei Bie
3 siblings, 0 replies; 13+ messages in thread
From: Tiwei Bie @ 2025-03-06 15:07 UTC (permalink / raw)
To: richard, anton.ivanov, johannes; +Cc: linux-um, Tiwei Bie
The ubd io thread and UML kernel thread share the same errno, which
can lead to conflicts when both call syscalls concurrently. Switch to
the pthread-based helper to address this issue.
Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
---
arch/um/drivers/ubd.h | 6 ++++--
arch/um/drivers/ubd_kern.c | 25 +++++++++++--------------
arch/um/drivers/ubd_user.c | 14 +++++++-------
3 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/arch/um/drivers/ubd.h b/arch/um/drivers/ubd.h
index f016fe15499f..2985c14661f4 100644
--- a/arch/um/drivers/ubd.h
+++ b/arch/um/drivers/ubd.h
@@ -7,8 +7,10 @@
#ifndef __UM_UBD_USER_H
#define __UM_UBD_USER_H
-extern int start_io_thread(unsigned long sp, int *fds_out);
-extern int io_thread(void *arg);
+#include <os.h>
+
+int start_io_thread(struct os_helper_thread **td_out, int *fd_out);
+void *io_thread(void *arg);
extern int kernel_fd;
extern int ubd_read_poll(int timeout);
diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 0b1e61f72fb3..4de6613e7468 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -474,12 +474,12 @@ static irqreturn_t ubd_intr(int irq, void *dev)
}
/* Only changed by ubd_init, which is an initcall. */
-static int io_pid = -1;
+static struct os_helper_thread *io_td;
static void kill_io_thread(void)
{
- if(io_pid != -1)
- os_kill_process(io_pid, 1);
+ if (io_td)
+ os_kill_helper_thread(io_td);
}
__uml_exitcall(kill_io_thread);
@@ -1104,8 +1104,8 @@ static int __init ubd_init(void)
late_initcall(ubd_init);
-static int __init ubd_driver_init(void){
- unsigned long stack;
+static int __init ubd_driver_init(void)
+{
int err;
/* Set by CONFIG_BLK_DEV_UBD_SYNC or ubd=sync.*/
@@ -1114,13 +1114,11 @@ static int __init ubd_driver_init(void){
/* Letting ubd=sync be like using ubd#s= instead of ubd#= is
* enough. So use anyway the io thread. */
}
- stack = alloc_stack(0, 0);
- io_pid = start_io_thread(stack + PAGE_SIZE, &thread_fd);
- if(io_pid < 0){
+ err = start_io_thread(&io_td, &thread_fd);
+ if (err < 0) {
printk(KERN_ERR
"ubd : Failed to start I/O thread (errno = %d) - "
- "falling back to synchronous I/O\n", -io_pid);
- io_pid = -1;
+ "falling back to synchronous I/O\n", -err);
return 0;
}
err = um_request_irq(UBD_IRQ, thread_fd, IRQ_READ, ubd_intr,
@@ -1496,12 +1494,11 @@ int kernel_fd = -1;
/* Only changed by the io thread. XXX: currently unused. */
static int io_count;
-int io_thread(void *arg)
+void *io_thread(void *arg)
{
int n, count, written, res;
- os_set_pdeathsig();
- os_fix_helper_signals();
+ os_fix_helper_thread_signals();
while(1){
n = bulk_req_safe_read(
@@ -1543,5 +1540,5 @@ int io_thread(void *arg)
} while (written < n);
}
- return 0;
+ return NULL;
}
diff --git a/arch/um/drivers/ubd_user.c b/arch/um/drivers/ubd_user.c
index b4f8b8e60564..c5e6545f6fcf 100644
--- a/arch/um/drivers/ubd_user.c
+++ b/arch/um/drivers/ubd_user.c
@@ -25,9 +25,9 @@
static struct pollfd kernel_pollfd;
-int start_io_thread(unsigned long sp, int *fd_out)
+int start_io_thread(struct os_helper_thread **td_out, int *fd_out)
{
- int pid, fds[2], err;
+ int fds[2], err;
err = os_pipe(fds, 1, 1);
if(err < 0){
@@ -47,14 +47,14 @@ int start_io_thread(unsigned long sp, int *fd_out)
goto out_close;
}
- pid = clone(io_thread, (void *) sp, CLONE_FILES | CLONE_VM, NULL);
- if(pid < 0){
- err = -errno;
- printk("start_io_thread - clone failed : errno = %d\n", errno);
+ err = os_run_helper_thread(td_out, io_thread, NULL);
+ if (err < 0) {
+ printk("%s - failed to run helper thread, err = %d\n",
+ __func__, -err);
goto out_close;
}
- return(pid);
+ return 0;
out_close:
os_close_file(fds[0]);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] um: Switch to the pthread-based helper in sigio workaround
2025-03-06 15:07 [PATCH v2 0/4] um: Make errno multi-thread safe Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 1/4] um: Add pthread-based helper support Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 2/4] um: ubd: Switch to the pthread-based helper Tiwei Bie
@ 2025-03-06 15:07 ` Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 4/4] um: Prohibit the VM_CLONE flag in run_helper_thread() Tiwei Bie
3 siblings, 0 replies; 13+ messages in thread
From: Tiwei Bie @ 2025-03-06 15:07 UTC (permalink / raw)
To: richard, anton.ivanov, johannes; +Cc: linux-um, Tiwei Bie
The write_sigio thread and UML kernel thread share the same errno,
which can lead to conflicts when both call syscalls concurrently.
Switch to the pthread-based helper to address this issue.
Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
---
arch/um/os-Linux/sigio.c | 44 +++++++++++++++++-----------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/arch/um/os-Linux/sigio.c b/arch/um/os-Linux/sigio.c
index 9aac8def4d63..61b348a2ea97 100644
--- a/arch/um/os-Linux/sigio.c
+++ b/arch/um/os-Linux/sigio.c
@@ -21,8 +21,7 @@
* Protected by sigio_lock(), also used by sigio_cleanup, which is an
* exitcall.
*/
-static int write_sigio_pid = -1;
-static unsigned long write_sigio_stack;
+static struct os_helper_thread *write_sigio_td;
/*
* These arrays are initialized before the sigio thread is started, and
@@ -48,15 +47,15 @@ static struct pollfds current_poll;
static struct pollfds next_poll;
static struct pollfds all_sigio_fds;
-static int write_sigio_thread(void *unused)
+static void *write_sigio_thread(void *unused)
{
struct pollfds *fds, tmp;
struct pollfd *p;
int i, n, respond_fd;
char c;
- os_set_pdeathsig();
- os_fix_helper_signals();
+ os_fix_helper_thread_signals();
+
fds = ¤t_poll;
while (1) {
n = poll(fds->poll, fds->used, -1);
@@ -98,7 +97,7 @@ static int write_sigio_thread(void *unused)
}
}
- return 0;
+ return NULL;
}
static int need_poll(struct pollfds *polls, int n)
@@ -152,11 +151,10 @@ static void update_thread(void)
return;
fail:
/* Critical section start */
- if (write_sigio_pid != -1) {
- os_kill_process(write_sigio_pid, 1);
- free_stack(write_sigio_stack, 0);
+ if (write_sigio_td) {
+ os_kill_helper_thread(write_sigio_td);
+ write_sigio_td = NULL;
}
- write_sigio_pid = -1;
close(sigio_private[0]);
close(sigio_private[1]);
close(write_sigio_fds[0]);
@@ -220,7 +218,7 @@ int __ignore_sigio_fd(int fd)
* sigio_cleanup has already run, then update_thread will hang
* or fail because the thread is no longer running.
*/
- if (write_sigio_pid == -1)
+ if (!write_sigio_td)
return -EIO;
for (i = 0; i < current_poll.used; i++) {
@@ -279,14 +277,14 @@ static void write_sigio_workaround(void)
int err;
int l_write_sigio_fds[2];
int l_sigio_private[2];
- int l_write_sigio_pid;
+ struct os_helper_thread *l_write_sigio_td;
/* We call this *tons* of times - and most ones we must just fail. */
sigio_lock();
- l_write_sigio_pid = write_sigio_pid;
+ l_write_sigio_td = write_sigio_td;
sigio_unlock();
- if (l_write_sigio_pid != -1)
+ if (l_write_sigio_td)
return;
err = os_pipe(l_write_sigio_fds, 1, 1);
@@ -312,7 +310,7 @@ static void write_sigio_workaround(void)
* Did we race? Don't try to optimize this, please, it's not so likely
* to happen, and no more than once at the boot.
*/
- if (write_sigio_pid != -1)
+ if (write_sigio_td)
goto out_free;
current_poll = ((struct pollfds) { .poll = p,
@@ -325,18 +323,15 @@ static void write_sigio_workaround(void)
memcpy(write_sigio_fds, l_write_sigio_fds, sizeof(l_write_sigio_fds));
memcpy(sigio_private, l_sigio_private, sizeof(l_sigio_private));
- write_sigio_pid = run_helper_thread(write_sigio_thread, NULL,
- CLONE_FILES | CLONE_VM,
- &write_sigio_stack);
-
- if (write_sigio_pid < 0)
+ err = os_run_helper_thread(&write_sigio_td, write_sigio_thread, NULL);
+ if (err < 0)
goto out_clear;
sigio_unlock();
return;
out_clear:
- write_sigio_pid = -1;
+ write_sigio_td = NULL;
write_sigio_fds[0] = -1;
write_sigio_fds[1] = -1;
sigio_private[0] = -1;
@@ -394,12 +389,11 @@ void maybe_sigio_broken(int fd)
static void sigio_cleanup(void)
{
- if (write_sigio_pid == -1)
+ if (!write_sigio_td)
return;
- os_kill_process(write_sigio_pid, 1);
- free_stack(write_sigio_stack, 0);
- write_sigio_pid = -1;
+ os_kill_helper_thread(write_sigio_td);
+ write_sigio_td = NULL;
}
__uml_exitcall(sigio_cleanup);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/4] um: Prohibit the VM_CLONE flag in run_helper_thread()
2025-03-06 15:07 [PATCH v2 0/4] um: Make errno multi-thread safe Tiwei Bie
` (2 preceding siblings ...)
2025-03-06 15:07 ` [PATCH v2 3/4] um: Switch to the pthread-based helper in sigio workaround Tiwei Bie
@ 2025-03-06 15:07 ` Tiwei Bie
3 siblings, 0 replies; 13+ messages in thread
From: Tiwei Bie @ 2025-03-06 15:07 UTC (permalink / raw)
To: richard, anton.ivanov, johannes; +Cc: linux-um, Tiwei Bie
Directly creating helper threads with VM_CLONE using clone can
compromise the thread safety of errno. Since all these helper
threads have been converted to use os_run_helper_thread(), let's
prevent using this flag in run_helper_thread().
Signed-off-by: Tiwei Bie <tiwei.btw@antgroup.com>
---
arch/um/os-Linux/helper.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c
index 5cb30773c511..d81d0a9363c7 100644
--- a/arch/um/os-Linux/helper.c
+++ b/arch/um/os-Linux/helper.c
@@ -122,6 +122,10 @@ int run_helper_thread(int (*proc)(void *), void *arg, unsigned int flags,
unsigned long stack, sp;
int pid, status, err;
+ /* To share memory space, use os_run_helper_thread() instead. */
+ if (flags & CLONE_VM)
+ return -EINVAL;
+
stack = alloc_stack(0, __uml_cant_sleep());
if (stack == 0)
return -ENOMEM;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] um: Add pthread-based helper support
2025-03-06 15:07 ` [PATCH v2 1/4] um: Add pthread-based helper support Tiwei Bie
@ 2025-03-18 13:06 ` Johannes Berg
2025-03-18 13:16 ` Johannes Berg
2025-03-18 15:00 ` Tiwei Bie
2025-03-19 9:11 ` Johannes Berg
1 sibling, 2 replies; 13+ messages in thread
From: Johannes Berg @ 2025-03-18 13:06 UTC (permalink / raw)
To: Tiwei Bie, richard, anton.ivanov; +Cc: linux-um
On Thu, 2025-03-06 at 23:07 +0800, Tiwei Bie wrote:
> Introduce a new set of utility functions that can be used to create
> pthread-based helpers. Helper threads created in this way will ensure
> thread safety for errno while sharing the same memory space.
Using pthreads seemed odd, but Benjamin argues that it's the only way to
get libc to really sort it all out, unless we never use libc syscall
functions, which is probably kind of unreasonable? Or maybe we could?
Either way though,
> +#include <pthread.h>
> + err = pthread_create(&td->handle, NULL, routine, arg);
if we're going to use pthread API, then we need to link against it?
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] um: Add pthread-based helper support
2025-03-18 13:06 ` Johannes Berg
@ 2025-03-18 13:16 ` Johannes Berg
2025-03-18 14:55 ` Tiwei Bie
2025-03-18 15:00 ` Tiwei Bie
1 sibling, 1 reply; 13+ messages in thread
From: Johannes Berg @ 2025-03-18 13:16 UTC (permalink / raw)
To: Tiwei Bie, richard, anton.ivanov; +Cc: linux-um
On Tue, 2025-03-18 at 14:06 +0100, Johannes Berg wrote:
> On Thu, 2025-03-06 at 23:07 +0800, Tiwei Bie wrote:
> > Introduce a new set of utility functions that can be used to create
> > pthread-based helpers. Helper threads created in this way will ensure
> > thread safety for errno while sharing the same memory space.
>
> Using pthreads seemed odd, but Benjamin argues that it's the only way to
> get libc to really sort it all out, unless we never use libc syscall
> functions, which is probably kind of unreasonable? Or maybe we could?
It's a long list of symbols that are needed:
https://p.sipsolutions.net/2f0c8e0de1e69147.txt
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] um: Add pthread-based helper support
2025-03-18 13:16 ` Johannes Berg
@ 2025-03-18 14:55 ` Tiwei Bie
2025-03-25 16:31 ` Benjamin Berg
0 siblings, 1 reply; 13+ messages in thread
From: Tiwei Bie @ 2025-03-18 14:55 UTC (permalink / raw)
To: Johannes Berg, richard, anton.ivanov; +Cc: linux-um
On 2025/3/18 21:16, Johannes Berg wrote:
> On Tue, 2025-03-18 at 14:06 +0100, Johannes Berg wrote:
>> On Thu, 2025-03-06 at 23:07 +0800, Tiwei Bie wrote:
>>> Introduce a new set of utility functions that can be used to create
>>> pthread-based helpers. Helper threads created in this way will ensure
>>> thread safety for errno while sharing the same memory space.
>>
>> Using pthreads seemed odd, but Benjamin argues that it's the only way to
>> get libc to really sort it all out, unless we never use libc syscall
>> functions, which is probably kind of unreasonable? Or maybe we could?
>
> It's a long list of symbols that are needed:
> https://p.sipsolutions.net/2f0c8e0de1e69147.txt
Thanks for the list! That's indeed a long one.
Regards,
Tiwei
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] um: Add pthread-based helper support
2025-03-18 13:06 ` Johannes Berg
2025-03-18 13:16 ` Johannes Berg
@ 2025-03-18 15:00 ` Tiwei Bie
1 sibling, 0 replies; 13+ messages in thread
From: Tiwei Bie @ 2025-03-18 15:00 UTC (permalink / raw)
To: Johannes Berg, richard, anton.ivanov; +Cc: linux-um
On 2025/3/18 21:06, Johannes Berg wrote:
> On Thu, 2025-03-06 at 23:07 +0800, Tiwei Bie wrote:
>> Introduce a new set of utility functions that can be used to create
>> pthread-based helpers. Helper threads created in this way will ensure
>> thread safety for errno while sharing the same memory space.
>
> Using pthreads seemed odd, but Benjamin argues that it's the only way to
> get libc to really sort it all out, unless we never use libc syscall
> functions, which is probably kind of unreasonable? Or maybe we could?
Thanks, Benjamin! :)
Yeah, it's also the only way I've figured out so far, unless we don't use libc.
>
>
> Either way though,
>
>> +#include <pthread.h>
>
>> + err = pthread_create(&td->handle, NULL, routine, arg);
>
> if we're going to use pthread API, then we need to link against it?
I had the same first thought, but scripts/link-vmlinux.sh already handles it:
https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/link-vmlinux.sh?h=v6.14-rc7#n85
Regards,
Tiwei
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] um: Add pthread-based helper support
2025-03-06 15:07 ` [PATCH v2 1/4] um: Add pthread-based helper support Tiwei Bie
2025-03-18 13:06 ` Johannes Berg
@ 2025-03-19 9:11 ` Johannes Berg
2025-03-19 12:07 ` Tiwei Bie
1 sibling, 1 reply; 13+ messages in thread
From: Johannes Berg @ 2025-03-19 9:11 UTC (permalink / raw)
To: Tiwei Bie, richard, anton.ivanov; +Cc: linux-um
On Thu, 2025-03-06 at 23:07 +0800, Tiwei Bie wrote:
> Introduce a new set of utility functions that can be used to create
> pthread-based helpers. Helper threads created in this way will ensure
> thread safety for errno while sharing the same memory space.
I'm not sure at the moment exactly what the issue is, but with the next
patch, I cannot shut down cleanly.
> +void os_kill_helper_thread(struct os_helper_thread *td)
> +{
> + pthread_kill(td->handle, SIGKILL);
This ends up killing everything.
I thought maybe it's because all the signals are blocked:
> +int os_run_helper_thread(struct os_helper_thread **td_out,
> + void *(*routine)(void *), void *arg)
> +{
> [...]
> + sigfillset(&sigset);
> + if (sigprocmask(SIG_SETMASK, &sigset, &oset) < 0) {
> + err = -errno;
> + kfree(td);
> + return err;
> + }
> +
> + err = pthread_create(&td->handle, NULL, routine, arg);
but removing SIGKILL from that set doesn't make a difference, and
perhaps this should be using pthread_sigmask(), e.g. if glibc doesn't
retrieve the values from the kernel but uses the ones last set by that
function?
Anyway, the issue is I cannot shut down, I get:
reboot: Power down
Killed
and the terminal isn't cleaned up, i.e. shutdown code isn't run.
Please check :)
johannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] um: Add pthread-based helper support
2025-03-19 9:11 ` Johannes Berg
@ 2025-03-19 12:07 ` Tiwei Bie
0 siblings, 0 replies; 13+ messages in thread
From: Tiwei Bie @ 2025-03-19 12:07 UTC (permalink / raw)
To: Johannes Berg, richard, anton.ivanov; +Cc: linux-um
On 2025/3/19 17:11, Johannes Berg wrote:
> On Thu, 2025-03-06 at 23:07 +0800, Tiwei Bie wrote:
>> +void os_kill_helper_thread(struct os_helper_thread *td)
>> +{
>> + pthread_kill(td->handle, SIGKILL);
>
> This ends up killing everything.
>
[...]
>
> Anyway, the issue is I cannot shut down, I get:
>
> reboot: Power down
> Killed
>
> and the terminal isn't cleaned up, i.e. shutdown code isn't run.
Whoops, I completely missed that.. :(
Thanks a lot for the test and analysis!
It seems that SIGKILL will always take the process down [1]. We cannot
use SIGKILL to kill helper threads anymore after switching to pthread.
[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/signal.c?h=v6.14-rc7#n1005
Regards,
Tiwei
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] um: Add pthread-based helper support
2025-03-18 14:55 ` Tiwei Bie
@ 2025-03-25 16:31 ` Benjamin Berg
2025-03-26 6:30 ` Tiwei Bie
0 siblings, 1 reply; 13+ messages in thread
From: Benjamin Berg @ 2025-03-25 16:31 UTC (permalink / raw)
To: Tiwei Bie, Johannes Berg, richard, anton.ivanov; +Cc: linux-um
On Tue, 2025-03-18 at 22:55 +0800, Tiwei Bie wrote:
> On 2025/3/18 21:16, Johannes Berg wrote:
> > On Tue, 2025-03-18 at 14:06 +0100, Johannes Berg wrote:
> > > On Thu, 2025-03-06 at 23:07 +0800, Tiwei Bie wrote:
> > > > Introduce a new set of utility functions that can be used to create
> > > > pthread-based helpers. Helper threads created in this way will ensure
> > > > thread safety for errno while sharing the same memory space.
> > >
> > > Using pthreads seemed odd, but Benjamin argues that it's the only way to
> > > get libc to really sort it all out, unless we never use libc syscall
> > > functions, which is probably kind of unreasonable? Or maybe we could?
> >
> > It's a long list of symbols that are needed:
> > https://p.sipsolutions.net/2f0c8e0de1e69147.txt
>
> Thanks for the list! That's indeed a long one.
So, if we dropped libc, then I think that would also imply dropping the
abstraction to support hosts other than Linux. And then the hard split
between user/kernel code might not be needed anymore as one would
ideally not rely on system headers at all.
I can imagine that parts of the code base would really benefit (e.g.
include handling). While other parts would obviously become more
complicated (e.g. parsing /proc/cpuinfo, mcontext handling).
It seems to me that the cleanup potential is likely big enough to make
it worthwhile.
Benjamin
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] um: Add pthread-based helper support
2025-03-25 16:31 ` Benjamin Berg
@ 2025-03-26 6:30 ` Tiwei Bie
0 siblings, 0 replies; 13+ messages in thread
From: Tiwei Bie @ 2025-03-26 6:30 UTC (permalink / raw)
To: Benjamin Berg, Johannes Berg, richard, anton.ivanov; +Cc: linux-um
On 2025/3/26 00:31, Benjamin Berg wrote:
> On Tue, 2025-03-18 at 22:55 +0800, Tiwei Bie wrote:
>> On 2025/3/18 21:16, Johannes Berg wrote:
>>> On Tue, 2025-03-18 at 14:06 +0100, Johannes Berg wrote:
>>>> On Thu, 2025-03-06 at 23:07 +0800, Tiwei Bie wrote:
>>>>> Introduce a new set of utility functions that can be used to create
>>>>> pthread-based helpers. Helper threads created in this way will ensure
>>>>> thread safety for errno while sharing the same memory space.
>>>>
>>>> Using pthreads seemed odd, but Benjamin argues that it's the only way to
>>>> get libc to really sort it all out, unless we never use libc syscall
>>>> functions, which is probably kind of unreasonable? Or maybe we could?
>>>
>>> It's a long list of symbols that are needed:
>>> https://p.sipsolutions.net/2f0c8e0de1e69147.txt
>>
>> Thanks for the list! That's indeed a long one.
>
> So, if we dropped libc, then I think that would also imply dropping the
> abstraction to support hosts other than Linux. And then the hard split
> between user/kernel code might not be needed anymore as one would
> ideally not rely on system headers at all.
>
> I can imagine that parts of the code base would really benefit (e.g.
> include handling). While other parts would obviously become more
> complicated (e.g. parsing /proc/cpuinfo, mcontext handling).
>
> It seems to me that the cleanup potential is likely big enough to make
> it worthwhile.
+1. By dropping libc entirely, we can achieve greater flexibility and
elegance by directly leveraging the Linux ABI. While this approach may
require considerable effort, the potential benefits seem worthwhile.
Regards,
Tiwei
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-03-26 6:36 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-06 15:07 [PATCH v2 0/4] um: Make errno multi-thread safe Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 1/4] um: Add pthread-based helper support Tiwei Bie
2025-03-18 13:06 ` Johannes Berg
2025-03-18 13:16 ` Johannes Berg
2025-03-18 14:55 ` Tiwei Bie
2025-03-25 16:31 ` Benjamin Berg
2025-03-26 6:30 ` Tiwei Bie
2025-03-18 15:00 ` Tiwei Bie
2025-03-19 9:11 ` Johannes Berg
2025-03-19 12:07 ` Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 2/4] um: ubd: Switch to the pthread-based helper Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 3/4] um: Switch to the pthread-based helper in sigio workaround Tiwei Bie
2025-03-06 15:07 ` [PATCH v2 4/4] um: Prohibit the VM_CLONE flag in run_helper_thread() Tiwei Bie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox