[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/2] virtio-scsi: replace VirtIOBlock dataplane_{start/starti
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH 1/2] virtio-scsi: replace VirtIOBlock dataplane_{start/starting/stopped} with enum |
Date: |
Mon, 8 Aug 2022 11:43:11 -0400 |
On Mon, Aug 08, 2022 at 05:41:46AM -0400, Emanuele Giuseppe Esposito wrote:
> Simplify the various dataplane stages in dataplane_start/stop by using
> a single enum instead of having multiple flags.
>
> Read/write the enum atomically, as it can be read also by iothread
> callbacks.
What guarantees that these relaxed loads/stores always produce
DATAPLANE_STARTING/STARTED in virtio_scsi_defer_to_dataplane() and not
an older value? Are there implicit memory barriers?
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
> hw/scsi/virtio-scsi-dataplane.c | 21 +++++++++------------
> hw/scsi/virtio-scsi.c | 10 ++++++----
> include/hw/virtio/virtio-scsi.h | 5 ++---
> include/hw/virtio/virtio.h | 7 +++++++
> 4 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> index a575c3f0cd..9ad73e3e19 100644
> --- a/hw/scsi/virtio-scsi-dataplane.c
> +++ b/hw/scsi/virtio-scsi-dataplane.c
> @@ -106,13 +106,12 @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
> VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
> VirtIOSCSI *s = VIRTIO_SCSI(vdev);
>
> - if (s->dataplane_started ||
> - s->dataplane_starting ||
> + if (qatomic_read(&s->dataplane_state) <= DATAPLANE_STARTED ||
It's not obvious that the STOPPING and STOPPED constants have a value
greater than STARTING and STARTED. It could be other way around too. It
would be safer to write the code so there are no assumptions about the
constants:
VirtIODataplaneStates state = qatomic_read(&s->dataplane_state);
if (state == DATAPLANE_STARTING || state == DATAPLANE_STARTED || ...)
signature.asc
Description: PGP signature