All Virtuozzo development lists (kernel + QEMU)
 help / color / mirror / Atom feed
From: Konstantin Khorenko <khorenko@virtuozzo.com>
To: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Cc: devel@openvz.org
Subject: Re: [Devel] [PATCH hci-8.1] drivers/vhost/blk: add support for worker ioctls
Date: Tue, 03 Nov 2026 18:08:03 +0100	[thread overview]
Message-ID: <179372568398.2524654.7562922233831913007.b4-review@b4> (raw)
In-Reply-To: <20261029110554.3315693-1-alexander.ivanov@virtuozzo.com>

I won't apply this version, please send v2 with the changes below.

On 10/29/26 12:05, Alexander Ivanov wrote:
> +	case VHOST_NEW_WORKER:
> +	case VHOST_FREE_WORKER:
> +	case VHOST_ATTACH_VRING_WORKER:
> +	case VHOST_GET_VRING_WORKER:
> +		mutex_lock(&blk->dev.mutex);
> +		ret = vhost_worker_ioctl(&blk->dev, ioctl, argp);
> +		mutex_unlock(&blk->dev.mutex);
> +		return ret;

- Put the VHOST_SET_NWORKERS revert (your v1 1/2) in front of this
  patch again, as one series.
- Fix the stranded-completion hang Sashiko found before this patch, in
  the same series.
- Say in the message that VHOST_SET_FORK_FROM_OWNER is needed before
  every VHOST_SET_OWNER.

VHOST_SET_NWORKERS (1491588e71daf) is still in the tree. Its workers
are written straight into vq->worker, and their attachment_cnt stays
0. So VHOST_FREE_WORKER frees a worker that a vq still uses, and the
next kick or completion writes into freed memory.

Before this patch the worker ioctls were not reachable in our kernel
at all, CONFIG_VHOST_SCSI is off. And our qemu (hci-dev) still calls
VHOST_SET_NWORKERS. In v1 the revert in 1/2 closed this hole, and it
got lost in the resend.

On the hang: the code path is old, but nobody runs vhost-blk in task
mode today, and in kthread mode a kill does not clear vq->worker. This
patch makes task mode the required setup, and killing a qemu with I/O
in flight is routine, so the fix has to land first, not later.
Mainline did the same for vhost-scsi.

> VHOST_NEW_WORKER needs the device in vhost_task mode, so userspace has
> to issue VHOST_SET_FORK_FROM_OWNER with VHOST_FORK_OWNER_TASK before
> VHOST_SET_OWNER: our fork_from_owner_default is kthread.

VHOST_RESET_OWNER puts fork_owner back to the default, and qemu resets
the owner on every device reset, so this is needed before every
VHOST_SET_OWNER, not only the first one.

-- 
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

      parent reply	other threads:[~2026-11-03 17:09 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 11:05 Alexander Ivanov
2026-10-29 11:10 ` no-reply
2026-10-29 12:16   ` Alexander Ivanov
2026-10-29 11:15 ` Andrey Zhadchenko
2026-11-03 17:08 ` Konstantin Khorenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=179372568398.2524654.7562922233831913007.b4-review@b4 \
    --to=khorenko@virtuozzo.com \
    --cc=alexander.ivanov@virtuozzo.com \
    --cc=devel@openvz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.