From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Venkateswararao Jujjuri (JV)" Subject: Re: [PATCH 2/2] [net/9p] Add waitq to VirtIO transport. Date: Tue, 05 Oct 2010 16:26:48 -0700 Message-ID: <4CABB438.4090109@linux.vnet.ibm.com> References: <1285977377-28736-1-git-send-email-jvrao@linux.vnet.ibm.com> <1285977377-28736-2-git-send-email-jvrao@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: v9fs-developer@lists.sourceforge.net, linux-fsdevel@vger.kernel.org To: "Aneesh Kumar K. V" Return-path: Received: from e9.ny.us.ibm.com ([32.97.182.139]:38275 "EHLO e9.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756333Ab0JEX0v (ORCPT ); Tue, 5 Oct 2010 19:26:51 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e9.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id o95N72e8026835 for ; Tue, 5 Oct 2010 19:07:02 -0400 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id o95NQobQ475150 for ; Tue, 5 Oct 2010 19:26:50 -0400 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id o95NQnT0004740 for ; Tue, 5 Oct 2010 17:26:50 -0600 In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 10/5/2010 7:45 AM, Aneesh Kumar K. V wrote: > On Fri, 1 Oct 2010 16:56:17 -0700, "Venkateswararao Jujjuri (JV)" wrote: >> If there is not enough space for the PDU on the VirtIO ring, current >> code returns -EIO propagating the error to user. >> >> This patch introduced a wqit_queue on the channel, and lets the process >> wait on this queue until VirtIO ring frees up. >> >> Signed-off-by: Venkateswararao Jujjuri >> --- >> net/9p/trans_virtio.c | 44 +++++++++++++++++++++++++++++++++++++------- >> 1 files changed, 37 insertions(+), 7 deletions(-) >> >> diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c >> index 0df84bf..2de5144 100644 >> --- a/net/9p/trans_virtio.c >> +++ b/net/9p/trans_virtio.c >> @@ -75,6 +75,8 @@ struct virtio_chan { >> struct p9_client *client; >> struct virtio_device *vdev; >> struct virtqueue *vq; >> + int ring_bufs_avail; >> + wait_queue_head_t *vc_wq; >> >> /* Scatterlist: can be too big for stack. */ >> struct scatterlist sg[VIRTQUEUE_NUM]; >> @@ -141,15 +143,21 @@ static void req_done(struct virtqueue *vq) >> do { >> spin_lock_irqsave(&chan->lock, flags); >> rc = virtqueue_get_buf(chan->vq,&len); >> - spin_unlock_irqrestore(&chan->lock, flags); >> >> if (rc != NULL) { >> + if (!chan->ring_bufs_avail) { >> + chan->ring_bufs_avail = 1; >> + wake_up(chan->vc_wq); >> + } >> + spin_unlock_irqrestore(&chan->lock, flags); >> P9_DPRINTK(P9_DEBUG_TRANS, ": rc %p\n", rc); >> P9_DPRINTK(P9_DEBUG_TRANS, ": lookup tag %d\n", >> rc->tag); >> req = p9_tag_lookup(chan->client, rc->tag); >> req->status = REQ_STATUS_RCVD; >> p9_client_cb(chan->client, req); >> + } else { >> + spin_unlock_irqrestore(&chan->lock, flags); >> } >> } while (rc != NULL); >> } >> @@ -212,6 +220,7 @@ p9_virtio_request(struct p9_client *client, struct p9_req_t *req) >> >> P9_DPRINTK(P9_DEBUG_TRANS, "9p debug: virtio request\n"); >> >> +req_retry: >> req->status = REQ_STATUS_SENT; >> >> spin_lock_irqsave(&chan->lock, flags); >> @@ -222,10 +231,21 @@ p9_virtio_request(struct p9_client *client, struct p9_req_t *req) >> >> err = virtqueue_add_buf(chan->vq, chan->sg, out, in, req->tc); >> if (err< 0) { >> - spin_unlock_irqrestore(&chan->lock, flags); >> - P9_DPRINTK(P9_DEBUG_TRANS, >> - "9p debug: virtio rpc add_buf returned failure"); >> - return -EIO; >> + if (err == -ENOSPC) { >> + chan->ring_bufs_avail = 0; >> + spin_unlock_irqrestore(&chan->lock, flags); >> + err = wait_event_interruptible_timeout(*chan->vc_wq, >> + chan->ring_bufs_avail, >> + HZ/4); > > Why do we need this to be _timeout ? Also if interrupted by a signal we do > want to return to user space with -EIO right ? Or we loop here always ? We are waiting for some other request to complete and the wait is outside the lock. Hence it is possible miss the wakwup...hence timed wait. No We loop here for ever as long as we get -ENOSPC is returned by virtqueue_add_buf. - JV > >> + P9_DPRINTK(P9_DEBUG_TRANS, "9p:Retry virtio request\n"); >> + goto req_retry; >> + } else { >> + spin_unlock_irqrestore(&chan->lock, flags); >> + P9_DPRINTK(P9_DEBUG_TRANS, >> + "9p debug: " >> + "virtio rpc add_buf returned failure"); >> + return -EIO; >> + } >> } > > -aneesh