Hi Alexandre
On Tue, Aug 2, 2022 at 11:12 AM Alexandre Courbot <acourbot@xxxxxxxxxxxx> wrote:
>
> Hi Suwan,
>
> Thanks for the fast reply!
>
> On Tue, Aug 2, 2022 at 1:55 AM Kim Suwan <suwan.kim027@xxxxxxxxx> wrote:
> >
> > Hi Alexandre,
> >
> > Thanks for reporting the issue.
> >
> > I think a possible scenario is that request fails at
> > virtio_queue_rqs() and it is passed to normal path (virtio_queue_rq).
> >
> > In this procedure, It is possible that blk_mq_start_request()
> > was called twice changing request state from MQ_RQ_IN_FLIGHT to
> > MQ_RQ_IN_FLIGHT.
>
> I have checked whether virtblk_prep_rq_batch() within
> virtio_queue_rqs() ever returns 0, and it looks like it never happens.
> So as far as I can tell all virtio_queue_rqs() are processed
> successfully - but maybe the request can also fail further down the
> line? Is there some extra instrumentation I can do to check that?
>
I'm looking at one more suspicious code.
If virtblk_add_req() fails within virtblk_add_req_batch(),
virtio_queue_rqs() passes the failed request to the normal path also
(virtio_queue_rq). Then, it can call blk_mq_start_request() twice.
Because I can't reproduce the issue on my vm, Could you test
the below patch?
I defer the blk_mq_start_request() call after virtblk_add_req()
to ensure that we call blk_mq_start_request() after all the
preparations finish.
> >
> > Could I know if the issue occurs every booting time?
>
> This is consistently happening at every boot, yes. However as I
> started adding printks here and there to try and figure out what was
> happening I have a harder time hitting that warning, suggesting a race
> condition somewhere...
>
> FWIW I have checked the status of the request that triggered the
> warning and as expected it is IN_FLIGHT.
>
I will look if there can be a race condition at virtio_queue_rqs().
Or maybe you can dump the two queues? (rqlist, requeue_list)
Regards,
Suwan Kim
---
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 6fc7850c2b0a..300c442d656d 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -314,8 +314,6 @@ static blk_status_t virtblk_prep_rq(struct
blk_mq_hw_ctx *hctx,
if (unlikely(status))
return status;
- blk_mq_start_request(req);
-
vbr->sg_table.nents = virtblk_map_data(hctx, req, vbr);
if (unlikely(vbr->sg_table.nents < 0)) {
virtblk_cleanup_cmd(req);
@@ -363,6 +361,8 @@ static blk_status_t virtio_queue_rq(struct
blk_mq_hw_ctx *hctx,
}
}
+ blk_mq_start_request(req);
+
if (bd->last && virtqueue_kick_prepare(vblk->vqs[qid].vq))
notify = true;
spin_unlock_irqrestore(&vblk->vqs[qid].lock, flags);
@@ -401,6 +401,8 @@ static bool virtblk_add_req_batch(struct virtio_blk_vq *vq,
virtblk_unmap_data(req, vbr);
virtblk_cleanup_cmd(req);
rq_list_add(requeue_list, req);
+ } else {
+ blk_mq_start_request(req);
}
}
