lkml.org 
[lkml]   [2022]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2] um: read multiple msg from virtio slave request fd
From
Are there any issues with that patch?
I would be happy to receive any comments or an acceptance :-D

Sorry for my former HTML-Email.

kind regards

Benjamin


Am 07.06.2022 um 13:27 schrieb Benjamin Beichler:
> If VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS is activated, the user mode
> linux virtio irq handler only read one msg from the corresponding socket.
> This creates issues, when the device emulation creates multiple call
> requests (e.g. for multiple virtqueues), as the socket buffer tend to fill
> up and the call requests are delayed.
>
> This creates a deadlock situation, when the device simulation blocks,
> because of sending a msg and the kernel side blocks because of
> synchronously waiting for an acknowledge of kick request.
>
> Actually inband notifications are meant to be used in combination with the
> time travel protocol, but it is not required, therefore this corner case
> needs to be handled.
>
> Anyways, in general it seems to be more natural to consume always all
> messages from a socket, instead of only a single one.
>
> Fixes: 2cd097ba8c05 ("um: virtio: Implement VHOST_USER_PROTOCOL_F_SLAVE_REQ")
> Signed-off-by: Benjamin Beichler <benjamin.beichler@uni-rostock.de>
> ---
> arch/um/drivers/virtio_uml.c | 71 +++++++++++++++++++-----------------
> 1 file changed, 37 insertions(+), 34 deletions(-)
>
> diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
> index 82ff3785bf69..3716c5f6f9aa 100644
> --- a/arch/um/drivers/virtio_uml.c
> +++ b/arch/um/drivers/virtio_uml.c
> @@ -374,45 +374,48 @@ static irqreturn_t vu_req_read_message(struct virtio_uml_device *vu_dev,
> u8 extra_payload[512];
> } msg;
> int rc;
> + irqreturn_t irq_rc = IRQ_NONE;
>
> - rc = vhost_user_recv_req(vu_dev, &msg.msg,
> - sizeof(msg.msg.payload) +
> - sizeof(msg.extra_payload));
> -
> - vu_dev->recv_rc = rc;
> - if (rc)
> - return IRQ_NONE;
> -
> - switch (msg.msg.header.request) {
> - case VHOST_USER_SLAVE_CONFIG_CHANGE_MSG:
> - vu_dev->config_changed_irq = true;
> - response = 0;
> - break;
> - case VHOST_USER_SLAVE_VRING_CALL:
> - virtio_device_for_each_vq((&vu_dev->vdev), vq) {
> - if (vq->index == msg.msg.payload.vring_state.index) {
> - response = 0;
> - vu_dev->vq_irq_vq_map |= BIT_ULL(vq->index);
> - break;
> + while (1) {
> + rc = vhost_user_recv_req(vu_dev, &msg.msg,
> + sizeof(msg.msg.payload) +
> + sizeof(msg.extra_payload));
> + if (rc)
> + break;
> +
> + switch (msg.msg.header.request) {
> + case VHOST_USER_SLAVE_CONFIG_CHANGE_MSG:
> + vu_dev->config_changed_irq = true;
> + response = 0;
> + break;
> + case VHOST_USER_SLAVE_VRING_CALL:
> + virtio_device_for_each_vq((&vu_dev->vdev), vq) {
> + if (vq->index == msg.msg.payload.vring_state.index) {
> + response = 0;
> + vu_dev->vq_irq_vq_map |= BIT_ULL(vq->index);
> + break;
> + }
> }
> + break;
> + case VHOST_USER_SLAVE_IOTLB_MSG:
> + /* not supported - VIRTIO_F_ACCESS_PLATFORM */
> + case VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG:
> + /* not supported - VHOST_USER_PROTOCOL_F_HOST_NOTIFIER */
> + default:
> + vu_err(vu_dev, "unexpected slave request %d\n",
> + msg.msg.header.request);
> }
> - break;
> - case VHOST_USER_SLAVE_IOTLB_MSG:
> - /* not supported - VIRTIO_F_ACCESS_PLATFORM */
> - case VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG:
> - /* not supported - VHOST_USER_PROTOCOL_F_HOST_NOTIFIER */
> - default:
> - vu_err(vu_dev, "unexpected slave request %d\n",
> - msg.msg.header.request);
> - }
> -
> - if (ev && !vu_dev->suspended)
> - time_travel_add_irq_event(ev);
>
> - if (msg.msg.header.flags & VHOST_USER_FLAG_NEED_REPLY)
> - vhost_user_reply(vu_dev, &msg.msg, response);
> + if (ev && !vu_dev->suspended)
> + time_travel_add_irq_event(ev);
>
> - return IRQ_HANDLED;
> + if (msg.msg.header.flags & VHOST_USER_FLAG_NEED_REPLY)
> + vhost_user_reply(vu_dev, &msg.msg, response);
> + irq_rc = IRQ_HANDLED;
> + };
> + /* mask EAGAIN as we try non-blocking read until socket is empty */
> + vu_dev->recv_rc = (rc == -EAGAIN) ? 0 : rc;
> + return irq_rc;
> }
>
> static irqreturn_t vu_req_interrupt(int irq, void *data)


--
M.Sc. Benjamin Beichler

Universität Rostock, Fakultät für Informatik und Elektrotechnik
Institut für Angewandte Mikroelektronik und Datentechnik

University of Rostock, Department of CS and EE
Institute of Applied Microelectronics and CE

Richard-Wagner-Straße 31
18119 Rostock
Deutschland/Germany

phone: +49 (0) 381 498 - 7278
email: Benjamin.Beichler@uni-rostock.de
www: http://www.imd.uni-rostock.de/
[unhandled content-type:application/pkcs7-signature]
\
 
 \ /
  Last update: 2022-07-27 22:28    [W:0.031 / U:3.700 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site