Skip to content

Commit

Permalink
Wait on individual buffers than the whole channel in v9fs_intr_complete
Browse files Browse the repository at this point in the history
In case of multiple concurrent vt9p_request() calls, there exists a situation
where one of the call dequeues all the posted responses before
virtqueue_intr_filter() was called. In such case virtqueue_intr() will
never be called and results in the other vt9p_requests() call returning EIO.

This can be demonstrated by the following sequence:
....
Thread A (on vCPU0)				Thread B (on vCPU1)
-------------------------------------------------------------------
mtx_lock(chan->mtx) {
    acquired;
}
						mtx_lock(chan->mtx) {
virtqueue_enqueue(vq0, reqA)
virtqueue_notify(vq0)
virtqueue_dequeue(vq0)): null
msleep() {
    mtx_unlock(chan->mtx);		    	    acquired;
						};
						virtqueue_enqueue(vq0, reqB)
						virtqueue_notify(vq0)
						virtqueue_dequeue(vq0): reqA; reqA->rc->tag = reqA->tc->tag
						reqB->rc->tag == P9_NOTAG
						virtqueue_dequeue(vq0): reqB; reqB->rc->tag = reqB->tc->tag
						reqB->rc->tag != P9_NOTAG
						mtx_unlock(chan->mtx)
						return
virtqueue_intr_filter(): FILTER_STRAY!!!
    mtx_lock(chan->mtx) {
	acquired;
    };
}
mtx_unlock(chan->mtx)
return EIO
....

This change moves the dequeue of responses to VirtIO-9P interrupt handler, and
wakes up the p9_req_t initiated by each vt9p_request() calls.

Tests were done on ACX5448 with the following case:
....

i=0
while [ $i -lt 3 ] ; do
	dd if=/dev/zero of=/hostvar/testdir/testfile.$i bs=1048576 count=128 status=progress &
	i=$((i+1))
done
wait
....

PR: 1727581
Change-Id: I189e88a623b6408aac9993c8ac43b1e57adc916e
  • Loading branch information
KaHoNg-JNPR authored and Kumara Babu Narayanaswamy committed Sep 20, 2023
1 parent e6bb5cf commit 731fffe
Showing 1 changed file with 41 additions and 25 deletions.
66 changes: 41 additions & 25 deletions sys/dev/virtio/9pnet/trans_virtio.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,32 @@ static unsigned int vt9p_ackmaxidle = 120;
SYSCTL_UINT(_vfs_9p, OID_AUTO, ackmaxidle, CTLFLAG_RW, &vt9p_ackmaxidle, 0,
"Maximum time request thread waits for ack from host");

/*
* Wait for completion of a p9 request.
*
* This routine will sleep and release the chan mtx during the period.
* chan mtx will be acquired again upon return.
*/
static int
vt9p_req_wait(struct vt9p_softc *chan, struct p9_req_t *req)
{
if (req->tc->tag != req->rc->tag) {
if (msleep(req, VT9P_MTX(chan), 0, "chan lock",
vt9p_ackmaxidle * hz)) {
/*
* Waited for 120s. No response from host.
* Can't wait for ever..
*/
p9_debug(ERROR, "Timeout after waiting %u seconds"
"for an ack from host\n", vt9p_ackmaxidle);
return (EIO);
}
KASSERT(req->tc->tag == req->rc->tag,
("Spurious event on p9 req"));
}
return (0);
}

/*
* Request handler. This is called for every request submitted to the host
* It basically maps the tc/rc buffers to sg lists and submits the requests
Expand All @@ -124,7 +150,6 @@ vt9p_request(struct p9_client *client, struct p9_req_t *req)
{
int error;
struct vt9p_softc *chan;
struct p9_req_t *curreq;
int readable, writable;
struct sglist *sg;
struct virtqueue *vq;
Expand All @@ -142,13 +167,15 @@ vt9p_request(struct p9_client *client, struct p9_req_t *req)
error = sglist_append(sg, req->tc->sdata, req->tc->size);
if (error != 0) {
p9_debug(ERROR, "sglist append failed\n");
VT9P_UNLOCK(chan);
return (error);
}
readable = sg->sg_nseg;

error = sglist_append(sg, req->rc->sdata, req->rc->capacity);
if (error != 0) {
p9_debug(ERROR, " sglist append failed\n");
VT9P_UNLOCK(chan);
return (error);
}
writable = sg->sg_nseg - readable;
Expand All @@ -167,34 +194,18 @@ vt9p_request(struct p9_client *client, struct p9_req_t *req)
goto req_retry;
} else {
p9_debug(ERROR, "virtio enuqueue failed \n");
VT9P_UNLOCK(chan);
return (EIO);
}
}

/* We have to notify */
virtqueue_notify(vq);

do {
curreq = virtqueue_dequeue(vq, NULL);
if (curreq == NULL) {
/* Nothing to dequeue, sleep until we have something */
if (msleep(chan, VT9P_MTX(chan), 0, "chan lock",
vt9p_ackmaxidle * hz)) {
/*
* Waited for 120s. No response from host.
* Can't wait for ever..
*/
p9_debug(ERROR, "Timeout after waiting %u seconds"
"for an ack from host\n", vt9p_ackmaxidle);
VT9P_UNLOCK(chan);
return (EIO);
}
} else {
cv_signal(&chan->submit_cv);
/* We dequeued something, update the reply tag */
curreq->rc->tag = curreq->tc->tag;
}
} while (req->rc->tag == P9_NOTAG);
error = vt9p_req_wait(chan, req);
if (error != 0) {
VT9P_UNLOCK(chan);
return (error);
}

VT9P_UNLOCK(chan);

Expand All @@ -206,22 +217,27 @@ vt9p_request(struct p9_client *client, struct p9_req_t *req)
/*
* Completion of the request from the virtqueue. This interrupt handler is
* setup at initialization and is called for every completing request. It
* just wakes up the sleeping submission thread.
* just wakes up the sleeping submission requests.
*/
static void
vt9p_intr_complete(void *xsc)
{
struct vt9p_softc *chan;
struct virtqueue *vq;
struct p9_req_t *curreq;

chan = (struct vt9p_softc *)xsc;
vq = chan->vt9p_vq;

p9_debug(TRANS, "Completing interrupt \n");

VT9P_LOCK(chan);
while ((curreq = virtqueue_dequeue(vq, NULL)) != NULL) {
curreq->rc->tag = curreq->tc->tag;
wakeup_one(curreq);
}
virtqueue_enable_intr(vq);
wakeup(chan);
cv_signal(&chan->submit_cv);
VT9P_UNLOCK(chan);
}

Expand Down

0 comments on commit 731fffe

Please sign in to comment.