Re: [PATCH] hw/char/virtio-serial-bus: fix: Unpop throttled VirtQueueElement to queue before discard vq data

[Stefan Hajnoczi]

On Fri, Jul 30, 2021 at 01:58:41AM +0000, AIERPATIJIANG1 [艾尔帕提江·阿布都赛买提] wrote:
> Ports enter a "throttled" state when writing to the chardev would block.
> The current output VirtQueueElement is kept around until the chardev
> becomes writable again.
> 
> Because closing the virtio serial device does not reset the queue, we cannot
> directly discard this element,  otherwise the control variables of the front
> and back ends of the queue are inconsistent such as used_index. We should 
> unpop the
> VirtQueueElement to queue, let discard_vq_data process it.

Laurent: Ping

> 
> The test environment:
> kernel: linux-5.12
> Qemu command:
> Qemu-system-x86 -machine pc,accel=kvm \
>     -cpu host,host-phys-bits \
>     -smp 4 \
>     -m 4G \
>     -kernel ./kernel \
>     -display none \
>     -nodefaults \
>     -serial mon:stdio \
>     -append "panic=1 no_timer_check noreplace-smp rootflags=data=ordered 
> rootfstype=ext4 console=ttyS0 reboot=k root=/dev/vda1 rw" \
>     -drive id=os,file=./disk,if=none \
>     -device virtio-blk-pci,drive=os \
>     -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 \
>     -chardev socket,id=charchannel0,path=/tmp/char-dev-test,server,nowait \
>   -device 
> virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
> 
> full up virtio queue after VM started:
> Cat /large-file > /dev/vport1p1
> 
> Host side:
> Open and close character device sockets repeatedly
> 
> After awhile we can’t write any request to /dev/vport1p1 at VM side, VM 
> kernel soft lockup at drivers/char/virtio_console.c: __send_to_port
> 
> 
> Signed-off-by: Arafatms <aierpatijiang1@kingsoft.com>
> 
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index dd6bc27b3b..36236defdf 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -150,8 +150,12 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice 
> *vdev)
> 
> static void discard_throttle_data(VirtIOSerialPort *port)
> {
> +    if (!virtio_queue_ready(port->ovq)) {
> +        return;
> +    }
> +
>      if (port->elem) {
> -        virtqueue_detach_element(port->ovq, port->elem, 0);
> +        virtqueue_unpop(port->ovq, port->elem, 0);
>          g_free(port->elem);
>          port->elem = NULL;
>      }
> 
> 
>

signature.asc
Description: PGP signature