On Thu, Dec 21, 2023 at 02:40:19PM +0100, Kevin Wolf wrote: > Am 20.12.2023 um 14:47 hat Stefan Hajnoczi geschrieben: > > Add the iothread-vq-mapping parameter to assign virtqueues to IOThreads. > > Store the vq:AioContext mapping in the new struct > > VirtIOBlockDataPlane->vq_aio_context[] field and refactor the code to > > use the per-vq AioContext instead of the BlockDriverState's AioContext. > > > > Reimplement --device virtio-blk-pci,iothread= and non-IOThread mode by > > assigning all virtqueues to the IOThread and main loop's AioContext in > > vq_aio_context[], respectively. > > > > The comment in struct VirtIOBlockDataPlane about EventNotifiers is > > stale. Remove it. > > > > Signed-off-by: Stefan Hajnoczi <[email protected]> > > > @@ -177,19 +238,13 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) > > > > trace_virtio_blk_data_plane_start(s); > > > > - r = blk_set_aio_context(s->conf->conf.blk, s->ctx, &local_err); > > + r = blk_set_aio_context(s->conf->conf.blk, s->vq_aio_context[0], > > + &local_err); > > if (r < 0) { > > error_report_err(local_err); > > goto fail_aio_context; > > } > > This doesn't really have to be an error any more, we'll just submit I/O > from any thread we want no matter what the home AioContext of the > BlockBackend is. > > So the only effect the blk_set_aio_context() has is that other users of > the image try to submit their requests from the same iothread as the > first virtqueue in the hope that this performs a bit better (maybe less > lock contention or whatever the idea was?)
Yes, I'll change this.
> > - /* Kick right away to begin processing requests already in vring */
> > - for (i = 0; i < nvqs; i++) {
> > - VirtQueue *vq = virtio_get_queue(s->vdev, i);
> > -
> > - event_notifier_set(virtio_queue_get_host_notifier(vq));
> > - }
> > -
> > /*
> > * These fields must be visible to the IOThread when it processes the
> > * virtqueue, otherwise it will think dataplane has not started yet.
> > @@ -206,8 +261,12 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
> > if (!blk_in_drain(s->conf->conf.blk)) {
> > for (i = 0; i < nvqs; i++) {
> > VirtQueue *vq = virtio_get_queue(s->vdev, i);
> > + AioContext *ctx = s->vq_aio_context[i];
> >
> > - virtio_queue_aio_attach_host_notifier(vq, s->ctx);
> > + /* Kick right away to begin processing requests already in
> > vring */
> > + event_notifier_set(virtio_queue_get_host_notifier(vq));
>
> The old code did this also for blk_in_drain() == true. Why don't we need
> it there any more? Should the 'if' move inside the loop just around
> attaching the notifier?
The answer is I'm not 100% sure. Your suggestion is safer, I'll do that.
Stefan
signature.asc
Description: PGP signature
