From mboxrd@z Thu Jan 1 00:00:00 1970 References: <1564465872-60404-1-git-send-email-bo.liu@linux.alibaba.com> <20190731174821.huiokivwch3ffglo@US-160370MP2.local> From: piaojun Message-ID: <5D42386B.7090208@huawei.com> Date: Thu, 1 Aug 2019 08:55:07 +0800 MIME-Version: 1.0 In-Reply-To: <20190731174821.huiokivwch3ffglo@US-160370MP2.local> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Subject: Re: [Virtio-fs] [PATCH] virtiofs: reduce lock contention on fpq->lock List-Id: Development discussions about virtio-fs List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: bo.liu@linux.alibaba.com Cc: virtio-fs@redhat.com On 2019/8/1 1:48, Liu Bo wrote: > Hi Jun, > > On Wed, Jul 31, 2019 at 08:05:03PM +0800, piaojun wrote: >> Hi liubo, >> >> This change has already committed in rhvgoyal's branch: >> >> https://github.com/rhvgoyal/linux/tree/virtio-fs-dev-5.1 >> > > Thanks for the link, the patch here does one more thing beyond Vivek's current > code, i.e. the lock protection around req is also removed as it's not necessary. Agreed, and it could save some CPU time. Thanks, Jun > > thanks, > -liubo > >> Thanks, >> Jun >> >> On 2019/7/30 13:51, Liu Bo wrote: >>> Right now within virtio_fs_requests_done_work(), virtqueue is protected by >>> both fsvq->lock and fpq->lock for processing both virtqueue and >>> fpq->processing_list, however, it is fine for them to be protected >>> separately. >>> >>> Also since %req has been removed from fpq->processing_list, no one except >>> request_wait_answer() is looking at this %req and request_wait_answer() >>> waits only on FINISH flag, it's OK to remove fpq->lock after %req is >>> dropped from the list. >>> >>> Signed-off-by: Liu Bo >>> --- >>> fs/fuse/virtio_fs.c | 13 ++++++------- >>> 1 file changed, 6 insertions(+), 7 deletions(-) >>> >>> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c >>> index 9b3f2e9..d88bb6c 100644 >>> --- a/fs/fuse/virtio_fs.c >>> +++ b/fs/fuse/virtio_fs.c >>> @@ -347,21 +347,22 @@ static void virtio_fs_requests_done_work(struct work_struct *work) >>> >>> /* Collect completed requests off the virtqueue */ >>> spin_lock(&fsvq->lock); >>> - /* grab fpq->lock as req is on fpq->processing list */ >>> - spin_lock(&fpq->lock); >>> do { >>> unsigned len; >>> >>> virtqueue_disable_cb(vq); >>> >>> while ((req = virtqueue_get_buf(vq, &len)) != NULL) { >>> + /* grab fpq->lock as req is on fpq->processing list */ >>> + spin_lock(&fpq->lock); >>> + >>> /* If !connected, req is freed by fuse_abort_conn. */ >>> - if (fpq->connected) { >>> + if (fpq->connected) >>> list_move_tail(&req->list, &reqs); >>> - } >>> + >>> + spin_unlock(&fpq->lock); >>> } >>> } while (!virtqueue_enable_cb(vq) && likely(!virtqueue_is_broken(vq))); >>> - spin_unlock(&fpq->lock); >>> spin_unlock(&fsvq->lock); >>> >>> /* End requests */ >>> @@ -373,10 +374,8 @@ static void virtio_fs_requests_done_work(struct work_struct *work) >>> >>> /* TODO zeroing? */ >>> >>> - spin_lock(&fpq->lock); >>> clear_bit(FR_SENT, &req->flags); >>> list_del_init(&req->list); >>> - spin_unlock(&fpq->lock); >>> >>> fuse_request_end(fc, req); >>> } >>> > . >