search for: clone_thread

...n), I think it's really wrong. The worker threads should show up as threads of the thing that started them, not as processes. So they should show up in 'ps' only when one of the "show threads" flag is set. But I suspect the fix is trivial: the virtio code should likely use CLONE_THREAD for the copy_process() it does. It should look more like "create_io_thread()" than "copy_process()", I think. For example, do virtio worker threads really want their own signals and files? That sounds wrong. create_io_thread() uses all of CLONE_FS|CLONE_FILES|CLONE_SIGHAND|C...

On 19.05.23 14:15, Christian Brauner wrote: > On Thu, May 18, 2023 at 10:25:11AM +0200, Christian Brauner wrote: >> On Wed, May 17, 2023 at 07:09:12PM -0500, Mike Christie wrote: >>> This patch allows the vhost and vhost_task code to use CLONE_THREAD, >>> CLONE_SIGHAND and CLONE_FILES. It's a RFC because I didn't do all the >>> normal testing, haven't coverted vsock and vdpa, and I know you guys >>> will not like the first patch. However, I think it better shows what >> >> Just to summarize the...

...uis wrote: >> On 19.05.23 14:15, Christian Brauner wrote: >>> On Thu, May 18, 2023 at 10:25:11AM +0200, Christian Brauner wrote: >>>> On Wed, May 17, 2023 at 07:09:12PM -0500, Mike Christie wrote: >>>>> This patch allows the vhost and vhost_task code to use CLONE_THREAD, >>>>> CLONE_SIGHAND and CLONE_FILES. It's a RFC because I didn't do all the >>>>> normal testing, haven't coverted vsock and vdpa, and I know you guys >>>>> will not like the first patch. However, I think it better shows what >>>&gt...

...uis wrote: >> On 19.05.23 14:15, Christian Brauner wrote: >>> On Thu, May 18, 2023 at 10:25:11AM +0200, Christian Brauner wrote: >>>> On Wed, May 17, 2023 at 07:09:12PM -0500, Mike Christie wrote: >>>>> This patch allows the vhost and vhost_task code to use CLONE_THREAD, >>>>> CLONE_SIGHAND and CLONE_FILES. It's a RFC because I didn't do all the >>>>> normal testing, haven't coverted vsock and vdpa, and I know you guys >>>>> will not like the first patch. However, I think it better shows what >>>&gt...

...e or add support for them. > > To fix both bugs, this switches the vhost task to be thread in the > process that does the VHOST_SET_OWNER ioctl, and has vhost_worker call > get_signal to support SIGKILL/SIGSTOP and freeze signals. Note that > SIGKILL/STOP support is required because CLONE_THREAD requires > CLONE_SIGHAND which requires those 2 signals to be suppported. > > This a modified version of patch originally written by Linus which > handles his review comment to himself to rename ignore_signals to > block_signals to better represent what it now does. And it includes...

...> The worker threads should show up as threads of the thing that started > them, not as processes. > > So they should show up in 'ps' only when one of the "show threads" flag is set. > > But I suspect the fix is trivial: the virtio code should likely use > CLONE_THREAD for the copy_process() it does. > > It should look more like "create_io_thread()" than "copy_process()", I think. > > For example, do virtio worker threads really want their own signals > and files? That sounds wrong. create_io_thread() uses all of > > C...

On Thu, Jun 1, 2023 at 6:47?AM Christian Brauner <brauner at kernel.org> wrote: > > @Mike, do you want to prepare an updated version of the temporary fix. > If @Linus prefers to just apply it directly he can just grab it from the > list rather than delaying it. Make sure to grab a Co-developed-by line > on this, @Mike. Yeah, let's apply the known "fix the immediate

On 6/6/23 7:16 AM, Oleg Nesterov wrote: > On 06/05, Mike Christie wrote: >> >> On 6/5/23 10:10 AM, Oleg Nesterov wrote: >>> On 06/03, michael.christie at oracle.com wrote: >>>> >>>> On 6/2/23 11:15 PM, Eric W. Biederman wrote: >>>> The problem is that as part of the flush the drivers/vhost/scsi.c code >>>> will wait for

On 05/23, Eric W. Biederman wrote: > > I want to point out that we need to consider not just SIGKILL, but > SIGABRT that causes a coredump, as well as the process peforming > an ordinary exit(2). All of which will cause get_signal to return > SIGKILL in this context. Yes, but probably SIGABRT/exit doesn't really differ from SIGKILL wrt vhost_worker(). > It is probably not

On 6/5/23 10:10 AM, Oleg Nesterov wrote: > On 06/03, michael.christie at oracle.com wrote: >> >> On 6/2/23 11:15 PM, Eric W. Biederman wrote: >> The problem is that as part of the flush the drivers/vhost/scsi.c code >> will wait for outstanding commands, because we can't free the device and >> it's resources before the commands complete or we will hit the

On 05/22, Mike Christie wrote: > > On 5/22/23 7:30 AM, Oleg Nesterov wrote: > >> + /* > >> + * When we get a SIGKILL our release function will > >> + * be called. That will stop new IOs from being queued > >> + * and check for outstanding cmd responses. It will then > >> + * call vhost_task_stop to tell us to return and exit. >

On 06/05, Mike Christie wrote: > > On 6/5/23 10:10 AM, Oleg Nesterov wrote: > > On 06/03, michael.christie at oracle.com wrote: > >> > >> On 6/2/23 11:15 PM, Eric W. Biederman wrote: > >> The problem is that as part of the flush the drivers/vhost/scsi.c code > >> will wait for outstanding commands, because we can't free the device and >

...rker->work_list is empty 3. Call vhost_task_stop() so why this sequence can't work if we turn vhost_dev_flush() into something like void vhost_dev_flush(struct vhost_dev *dev) { struct vhost_flush_struct flush; if (dev->worker) { // this assumes that vhost_task_create() uses CLONE_THREAD if (same_thread_group(current, dev->worker->vtsk->task)) { ... run the pending callbacks ... return; } // this is what we currently have init_completion(&flush.wait_event); vhost_work_init(&flush.work, vhost_flush_work); vhost_work_queue(dev, &flus...

Oleg Nesterov <oleg at redhat.com> writes: > On 05/22, Oleg Nesterov wrote: >> >> Right now I think that "int dead" should die, > > No, probably we shouldn't call get_signal() if we have already > dequeued SIGKILL. Very much agreed. It is one thing to add a patch to move do_exit out of get_signal. It is another to keep calling get_signal after that.

On 05/31, Jason Wang wrote: > > On Wed, May 31, 2023 at 3:25?PM Oleg Nesterov <oleg at redhat.com> wrote: > > > > On 05/31, Jason Wang wrote: > > > > > > ? 2023/5/23 20:15, Oleg Nesterov ??: > > > > > > > > /* make sure flag is seen after deletion */ > > > > smp_wmb(); > > > >

On Wed, May 31, 2023 at 5:14?PM Oleg Nesterov <oleg at redhat.com> wrote: > > On 05/31, Jason Wang wrote: > > > > On Wed, May 31, 2023 at 3:25?PM Oleg Nesterov <oleg at redhat.com> wrote: > > > > > > On 05/31, Jason Wang wrote: > > > > > > > > ? 2023/5/23 20:15, Oleg Nesterov ??: > > > > > > > > >

...g scripts. 2. vhost_tasks didn't disable or add support for freeze requests. The following patches fix these issues by making the vhost_task task a thread under the process that did the VHOST_SET_OWNER and uses get_signal() to handle freeze and SIGSTOP/KILL signals which is required when using CLONE_THREAD (really CLONE_THREAD requires CLONE_SIGHAND which requires SIGKILL/STOP to be supported).

On 05/22, Oleg Nesterov wrote: > > Right now I think that "int dead" should die, No, probably we shouldn't call get_signal() if we have already dequeued SIGKILL. > but let me think tomorrow. May be something like this... I don't like it but I can't suggest anything better right now. bool killed = false; for (;;) { ... node =

Hi Mike, sorry, but somehow I can't understand this patch... I'll try to read it with a fresh head on Weekend, but for example, On 06/01, Mike Christie wrote: > > static int vhost_task_fn(void *data) > { > struct vhost_task *vtsk = data; > - int ret; > + bool dead = false; > + > + for (;;) { > + bool did_work; > + > + /* mb paired w/

...s's didn't disable or add support for them. To fix both bugs, this switches the vhost task to be thread in the process that does the VHOST_SET_OWNER ioctl, and has vhost_worker call get_signal to support SIGKILL/SIGSTOP and freeze signals. Note that SIGKILL/STOP support is required because CLONE_THREAD requires CLONE_SIGHAND which requires those 2 signals to be supported. This is a modified version of the patch written by Mike Christie <michael.christie at oracle.com> which was a modified version of patch originally written by Linus. Much of what depended upon PF_IO_WORKER now depends on...