Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-08-13 Thread michael . christie
On 8/13/23 2:01 PM, Michael S. Tsirkin wrote: > On Fri, Aug 11, 2023 at 01:51:36PM -0500, Mike Christie wrote: >> On 8/10/23 1:57 PM, Michael S. Tsirkin wrote: >>> On Sat, Jul 22, 2023 at 11:03:29PM -0500, michael.chris...@oracle.com wrote: On 7/20/23 8:06 AM, Michael S. Tsirkin wrote: >

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-08-13 Thread Michael S. Tsirkin
On Fri, Aug 11, 2023 at 01:51:36PM -0500, Mike Christie wrote: > On 8/10/23 1:57 PM, Michael S. Tsirkin wrote: > > On Sat, Jul 22, 2023 at 11:03:29PM -0500, michael.chris...@oracle.com wrote: > >> On 7/20/23 8:06 AM, Michael S. Tsirkin wrote: > >>> On Thu, Feb 02, 2023 at 05:25:17PM -0600, Mike

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-08-11 Thread Mike Christie
On 8/10/23 1:57 PM, Michael S. Tsirkin wrote: > On Sat, Jul 22, 2023 at 11:03:29PM -0500, michael.chris...@oracle.com wrote: >> On 7/20/23 8:06 AM, Michael S. Tsirkin wrote: >>> On Thu, Feb 02, 2023 at 05:25:17PM -0600, Mike Christie wrote: For vhost workers we use the kthread API which

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-08-10 Thread Michael S. Tsirkin
On Sat, Jul 22, 2023 at 11:03:29PM -0500, michael.chris...@oracle.com wrote: > On 7/20/23 8:06 AM, Michael S. Tsirkin wrote: > > On Thu, Feb 02, 2023 at 05:25:17PM -0600, Mike Christie wrote: > >> For vhost workers we use the kthread API which inherit's its values from > >> and checks against the

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-07-23 Thread Michael S. Tsirkin
On Sat, Jul 22, 2023 at 11:03:29PM -0500, michael.chris...@oracle.com wrote: > On 7/20/23 8:06 AM, Michael S. Tsirkin wrote: > > On Thu, Feb 02, 2023 at 05:25:17PM -0600, Mike Christie wrote: > >> For vhost workers we use the kthread API which inherit's its values from > >> and checks against the

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-07-22 Thread michael . christie
On 7/20/23 8:06 AM, Michael S. Tsirkin wrote: > On Thu, Feb 02, 2023 at 05:25:17PM -0600, Mike Christie wrote: >> For vhost workers we use the kthread API which inherit's its values from >> and checks against the kthreadd thread. This results in the wrong RLIMITs >> being checked, so while tools

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-07-20 Thread Michael S. Tsirkin
On Thu, Feb 02, 2023 at 05:25:17PM -0600, Mike Christie wrote: > For vhost workers we use the kthread API which inherit's its values from > and checks against the kthreadd thread. This results in the wrong RLIMITs > being checked, so while tools like libvirt try to control the number of > threads

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-17 Thread Mike Christie
On 5/17/23 12:09 PM, Oleg Nesterov wrote: > IIUC, Mike is going to send the next version? So I think we can delay > the further discussions until then. Yeah, I'm working on a version that supports signals so it will be easier to discuss with the vhost devs and you, Christian and Eric.

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-17 Thread Oleg Nesterov
On 05/16, Eric W. Biederman wrote: > > Oleg Nesterov writes: > > >> There is this bit in complete_signal when SIGKILL is delivered to any > >> thread in the process. > >> > >>t = p; > >>do { > >>task_clear_jobctl_pending(t, > >>

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-16 Thread Eric W. Biederman
Oleg Nesterov writes: > On 05/16, Eric W. Biederman wrote: >> >> A kernel thread can block SIGKILL and that is supported. >> >> For a thread that is part of a process you can't block SIGKILL when the >> task is part of a user mode process. > > Or SIGSTOP. Another thread can call

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-16 Thread Oleg Nesterov
On 05/16, Eric W. Biederman wrote: > > A kernel thread can block SIGKILL and that is supported. > > For a thread that is part of a process you can't block SIGKILL when the > task is part of a user mode process. Or SIGSTOP. Another thread can call do_signal_stop()->signal_wake_up/etc. > There is

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-16 Thread Eric W. Biederman
Linus Torvalds writes: > On Mon, May 15, 2023 at 3:23 PM Mike Christie > wrote: >> >> The vhost layer really doesn't want any signals and wants to work like >> kthreads >> for that case. To make it really simple can we do something like this where >> it >> separates user and io worker

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-16 Thread Mike Christie
On 5/16/23 3:39 AM, Christian Brauner wrote: > On Mon, May 15, 2023 at 05:23:12PM -0500, Mike Christie wrote: >> On 5/15/23 10:44 AM, Linus Torvalds wrote: >>> On Mon, May 15, 2023 at 7:23 AM Christian Brauner >>> wrote: So I think we will be able to address (1) and (2) by making vhost

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-16 Thread Oleg Nesterov
On 05/15, Mike Christie wrote: > > --- a/kernel/vhost_task.c > +++ b/kernel/vhost_task.c > @@ -75,13 +75,13 @@ struct vhost_task *vhost_task_create(int (*fn)(void *), > void *arg, >const char *name) > { > struct kernel_clone_args args = { > -

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-16 Thread Oleg Nesterov
On 05/15, Mike Christie wrote: > > Oleg and Christian, > > > Below is an updated patch that doesn't check for PF_USER_WORKER in the > signal.c code and instead will check for if we have blocked the signal. Looks like I need to read the whole series... will try tomorrow. > --- a/kernel/fork.c >

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-15 Thread Mike Christie
On 5/15/23 5:54 PM, Linus Torvalds wrote: > On Mon, May 15, 2023 at 3:23 PM Mike Christie > wrote: >> >> The vhost layer really doesn't want any signals and wants to work like >> kthreads >> for that case. To make it really simple can we do something like this where >> it >> separates user and

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-15 Thread Linus Torvalds
On Mon, May 15, 2023 at 3:23 PM Mike Christie wrote: > > The vhost layer really doesn't want any signals and wants to work like > kthreads > for that case. To make it really simple can we do something like this where it > separates user and io worker behavior where the major diff is how they

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-15 Thread Mike Christie
On 5/15/23 10:44 AM, Linus Torvalds wrote: > On Mon, May 15, 2023 at 7:23 AM Christian Brauner wrote: >> >> So I think we will be able to address (1) and (2) by making vhost tasks >> proper threads and blocking every signal except for SIGKILL and SIGSTOP >> and then having vhost handle

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-15 Thread Linus Torvalds
On Mon, May 15, 2023 at 8:54 AM Linus Torvalds wrote: > > Blush. I decided to build-test it, and then forgot to attach it. Here. Btw, if this tests out good and we end up doing this, I think we should also just rename that '.ignore_signals' bitfield to '.block_signals' to actually match what it

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-15 Thread Linus Torvalds
On Mon, May 15, 2023 at 8:52 AM Jens Axboe wrote: > > Only potential downside is that it does make file references more > expensive for other syscalls, since you now have a shared file table. > But probably not something to worry about here? Would the vhost user worker user processes ever be

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-15 Thread Linus Torvalds
On Mon, May 15, 2023 at 8:52 AM Jens Axboe wrote: > > On 5/15/23 9:44?AM, Linus Torvalds wrote: > > > > So I think the patch should just look something like the attached. > > Mike, can you test this on whatever vhost test-suite? > > Seems like that didn't get attached... Blush. I decided to

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-15 Thread Jens Axboe
On 5/15/23 9:44?AM, Linus Torvalds wrote: > On Mon, May 15, 2023 at 7:23?AM Christian Brauner wrote: >> >> So I think we will be able to address (1) and (2) by making vhost tasks >> proper threads and blocking every signal except for SIGKILL and SIGSTOP >> and then having vhost handle

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-15 Thread Linus Torvalds
On Mon, May 15, 2023 at 7:23 AM Christian Brauner wrote: > > So I think we will be able to address (1) and (2) by making vhost tasks > proper threads and blocking every signal except for SIGKILL and SIGSTOP > and then having vhost handle get_signal() - as you mentioned - the same > way io uring

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-13 Thread Linus Torvalds
On Sat, May 13, 2023 at 7:39 AM Thorsten Leemhuis wrote: > > Jumping in here, as I found another problem with that patch: it broke > s2idle on my laptop when a qemu-kvm VM is running, as freezing user > space processes now fails for me: Hmm. kthreads have PF_NOFREEZE by default, which is

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-13 Thread Thorsten Leemhuis
[CCing the regression list] On 06.05.23 00:37, Mike Christie wrote: > On 5/5/23 1:22 PM, Linus Torvalds wrote: >> On Fri, May 5, 2023 at 6:40 AM Nicolas Dichtel >> wrote: >>> >>> Is this an intended behavior? >>> This breaks some of our scripts. Jumping in here, as I found another problem with

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-05 Thread Linus Torvalds
On Fri, May 5, 2023 at 3:38 PM Mike Christie wrote: > > If it's ok to change the behavior of "ps -u root", then we can do this patch: I think this is the right thing to do. Making the user worker threads show up as threads with the vhost process as the parent really seems like a much better

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-05 Thread Mike Christie
On 5/5/23 1:22 PM, Linus Torvalds wrote: > On Fri, May 5, 2023 at 6:40 AM Nicolas Dichtel > wrote: >> >> Is this an intended behavior? >> This breaks some of our scripts. > > It doesn't just break your scripts (which counts as a regression), I > think it's really wrong. > > The worker threads

Re: [PATCH v11 8/8] vhost: use vhost_tasks for worker threads

2023-05-05 Thread Linus Torvalds
On Fri, May 5, 2023 at 6:40 AM Nicolas Dichtel wrote: > > Is this an intended behavior? > This breaks some of our scripts. It doesn't just break your scripts (which counts as a regression), I think it's really wrong. The worker threads should show up as threads of the thing that started them,