[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 0/2] virtio-blk and scsi: replace dataplane_{start/stopping/s

From: Stefan Hajnoczi
Subject: Re: [PATCH 0/2] virtio-blk and scsi: replace dataplane_{start/stopping/started}
Date: Mon, 8 Aug 2022 11:52:39 -0400

On Mon, Aug 08, 2022 at 05:41:45AM -0400, Emanuele Giuseppe Esposito wrote:
> The way the dataplane stages at startup and stop are monitored is unnecessary
> complicated. In virtio-scsi we have dataplane_started, dataplane_starting and
> dataplane_stopping in VirtIOSCSI.
> In virtio-blk we have dataplene_started in VirtIOBlock, and starting and 
> stopping
> in VirtIOBlockDataPlane.
> Just replace all these flags with an atomic enum.
> Based-on: 20220803162824.948023-1-stefanha@redhat.com

As mentioned on IRC, I don't think it's useful to combine these fields
into a state machine because they serve different purposes
(starting/stopping prevents re-entrancy, started/stopped tracks whether
dataplane is enabled, and fenced tracks whether dataplane is broken).

Combining them all makes it harder to refactor those separate concerns.
In the future it would be nice to refactor away all this state and have
stateless IOThread support (I haven't figured out whether that's
possible yet).

I'm not against merging this, but I don't think using an enum is an
improvement - it's just different.

Regarding thread-safety, we need to guarantee that the state stored by a
QEMU thread is visible to the IOThread. This patch series doesn't really
do this: relaxed loads/stores aren't guaranteed to be visible to the
other thread. I comment on this in the patch: there needs to be at least
a comment or maybe something extra, like a memory barrier or stronger
atomic operation, to make the change to atomics worthwhile.

Attachment: signature.asc
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]