Skip to content

Commit

Permalink
vtblk: Invoke busdma completion callbacks when polling
Browse files Browse the repository at this point in the history
vtblk_poll_request() is used for kernel dumps and for fetching the block
device's identifier string during device probing.  In the latter case,
it was not calling bus_dmamap_sync() after completing the I/O, but this
is required in general.

Thus:
- Factor out per-request code from vtblk_queue_completed().
- Use it in vtblk_poll_request() once virtqueue_poll() finishes.
- While here, assert that virtqueue_poll() returns the request that we
  expect.

Reported by:	KMSAN
Fixes:		782105f ("vtblk: Use busdma")
Reviewed by:	cperciva, imp
Sponsored by:	Klara, Inc.
Sponsored by:	Juniper Networks, Inc.
Differential Revision:	https://reviews.freebsd.org/D45665
  • Loading branch information
markjdb committed Jul 4, 2024
1 parent a2e65d4 commit b1dd067
Showing 1 changed file with 39 additions and 27 deletions.
66 changes: 39 additions & 27 deletions sys/dev/virtio/block/virtio_blk.c
Original file line number Diff line number Diff line change
Expand Up @@ -1177,38 +1177,45 @@ vtblk_request_error(struct vtblk_request *req)
return (error);
}

static struct bio *
vtblk_queue_complete_one(struct vtblk_softc *sc, struct vtblk_request *req)
{
struct bio *bp;

if (sc->vtblk_req_ordered != NULL) {
MPASS(sc->vtblk_req_ordered == req);
sc->vtblk_req_ordered = NULL;
}

bp = req->vbr_bp;
if (req->vbr_mapp != NULL) {
switch (bp->bio_cmd) {
case BIO_READ:
bus_dmamap_sync(sc->vtblk_dmat, req->vbr_mapp,
BUS_DMASYNC_POSTREAD);
bus_dmamap_unload(sc->vtblk_dmat, req->vbr_mapp);
break;
case BIO_WRITE:
bus_dmamap_sync(sc->vtblk_dmat, req->vbr_mapp,
BUS_DMASYNC_POSTWRITE);
bus_dmamap_unload(sc->vtblk_dmat, req->vbr_mapp);
break;
}
}
bp->bio_error = vtblk_request_error(req);
return (bp);
}

static void
vtblk_queue_completed(struct vtblk_softc *sc, struct bio_queue *queue)
{
struct vtblk_request *req;
struct bio *bp;

while ((req = virtqueue_dequeue(sc->vtblk_vq, NULL)) != NULL) {
if (sc->vtblk_req_ordered != NULL) {
MPASS(sc->vtblk_req_ordered == req);
sc->vtblk_req_ordered = NULL;
}
bp = vtblk_queue_complete_one(sc, req);

bp = req->vbr_bp;
if (req->vbr_mapp != NULL) {
switch (bp->bio_cmd) {
case BIO_READ:
bus_dmamap_sync(sc->vtblk_dmat, req->vbr_mapp,
BUS_DMASYNC_POSTREAD);
bus_dmamap_unload(sc->vtblk_dmat,
req->vbr_mapp);
break;
case BIO_WRITE:
bus_dmamap_sync(sc->vtblk_dmat, req->vbr_mapp,
BUS_DMASYNC_POSTWRITE);
bus_dmamap_unload(sc->vtblk_dmat,
req->vbr_mapp);
break;
}
}
bp->bio_error = vtblk_request_error(req);
TAILQ_INSERT_TAIL(queue, bp, bio_queue);

vtblk_request_enqueue(sc, req);
}
}
Expand Down Expand Up @@ -1412,8 +1419,6 @@ vtblk_ident(struct vtblk_softc *sc)
error = vtblk_poll_request(sc, req);
VTBLK_UNLOCK(sc);

vtblk_request_enqueue(sc, req);

if (error) {
device_printf(sc->vtblk_dev,
"error getting device identifier: %d\n", error);
Expand All @@ -1423,7 +1428,9 @@ vtblk_ident(struct vtblk_softc *sc)
static int
vtblk_poll_request(struct vtblk_softc *sc, struct vtblk_request *req)
{
struct vtblk_request *req1 __diagused;
struct virtqueue *vq;
struct bio *bp;
int error;

vq = sc->vtblk_vq;
Expand All @@ -1436,13 +1443,18 @@ vtblk_poll_request(struct vtblk_softc *sc, struct vtblk_request *req)
return (error);

virtqueue_notify(vq);
virtqueue_poll(vq, NULL);
req1 = virtqueue_poll(vq, NULL);
KASSERT(req == req1,
("%s: polling completed %p not %p", __func__, req1, req));

error = vtblk_request_error(req);
bp = vtblk_queue_complete_one(sc, req);
error = bp->bio_error;
if (error && bootverbose) {
device_printf(sc->vtblk_dev,
"%s: IO error: %d\n", __func__, error);
}
if (req != &sc->vtblk_dump_request)
vtblk_request_enqueue(sc, req);

return (error);
}
Expand Down

0 comments on commit b1dd067

Please sign in to comment.