On Fri, Jul 08, 2022 at 01:22:58PM +0200, Emanuele Giuseppe Esposito wrote:
>
>
> Am 08/07/2022 um 11:33 schrieb Emanuele Giuseppe Esposito:
> >
> >
> > Am 05/07/2022 um 16:45 schrieb Stefan Hajnoczi:
> >> On Thu, Jun 09, 2022 at 10:37:26AM -0400, Emanuele Giuseppe Esposito wrote:
> >>> @@ -946,17 +955,20 @@ static void virtio_blk_reset(VirtIODevice *vdev)
> >>> * stops all Iothreads.
> >>> */
> >>> blk_drain(s->blk);
> >>> + aio_context_release(ctx);
> >>>
> >>> /* We drop queued requests after blk_drain() because blk_drain()
> >>> itself can
> >>> * produce them. */
> >>> + qemu_mutex_lock(&s->req_mutex);
> >>> while (s->rq) {
> >>> req = s->rq;
> >>> s->rq = req->next;
> >>> + qemu_mutex_unlock(&s->req_mutex);
> >>> virtqueue_detach_element(req->vq, &req->elem, 0);
> >>> virtio_blk_free_request(req);
> >>> + qemu_mutex_lock(&s->req_mutex);
> >>
> >> Why is req_mutex dropped temporarily? At this point we don't really need
> >> the req_mutex (all I/O should be stopped and drained), but maybe we
> >> should do:
> >
> > Agree that maybe it is not useful to drop the mutex temporarily.
> >
> > Regarding why req_mutex is not needed, yes I guess it isn't. Should I
> > get rid of this hunk at all, and maybe leave a comment like "no
> > synchronization needed, due to drain + ->stop_ioeventfd()"?
>
> Actually, regarding this, I found why I added the lock:
>
> https://patchew.org/QEMU/20220426085114.199647-1-eespo...@redhat.com/#584d7d1a-94cc-9ebb-363b-2fddb8d79...@redhat.com
>
> So maybe it's better to add it.I don't see anything obvious in Paolo's email that you linked. I think he was saying it's safest to use a lock but not that it's necessary. Can you clarify what you mean? Stefan
signature.asc
Description: PGP signature
