diff mbox series

[2/2] virtiofs: reduce lock contention on fpq->lock

Message ID 20210812054618.26057-3-jefflexu@linux.alibaba.com (mailing list archive)
State New, archived
Headers show
Series virtiofs: miscellaneous fixes | expand

Commit Message

Jingbo Xu Aug. 12, 2021, 5:46 a.m. UTC 
From: Liu Bo <bo.liu@linux.alibaba.com>

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 <bo.liu@linux.alibaba.com>
Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
---
 fs/fuse/virtio_fs.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Miklos Szeredi Sept. 7, 2021, 8:57 a.m. UTC | #1 
On Thu, 12 Aug 2021 at 07:46, Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
>
> From: Liu Bo <bo.liu@linux.alibaba.com>
>
> 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.

I'll accept a patch to remove FR_SENT completely from virtiofs.

This flag is used for queuing interrupts but interrupts are not yet
implemented in virtiofs.    When blocking lock support is added the
interrupt handling needs to be properly designed.

Thanks,
Miklos
Vivek Goyal Sept. 7, 2021, 6:10 p.m. UTC | #2 
On Tue, Sep 07, 2021 at 10:57:07AM +0200, Miklos Szeredi wrote:
> On Thu, 12 Aug 2021 at 07:46, Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
> >
> > From: Liu Bo <bo.liu@linux.alibaba.com>
> >
> > 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.
> 
> I'll accept a patch to remove FR_SENT completely from virtiofs.
> 

Recently I was also looking at FR_SENT flag and was wondering if it
is atomic bit flag, then why do we need to take spin lock around it.
Probably we need just some barrier if code needs it but not necessarily
any lock.

But I agree that FR_SENT seems not usable from virtiofs point of view
as we don't have support for interrupt request.

> This flag is used for queuing interrupts but interrupts are not yet
> implemented in virtiofs.    When blocking lock support is added the
> interrupt handling needs to be properly designed.

Hmm.., I did not think about this. I was getting ready to post patches
for blocking posix locks but it does not have any support for interrupting
the locking request (either blocked or queued).

Is implementing interrupt support a requirement for getting blocking
posix lock patches in?

Thanks
Vivek
diff mbox series

Patch

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 0050132e2787..7dbf5502c57e 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -557,7 +557,6 @@  static void copy_args_from_argbuf(struct fuse_args *args, struct fuse_req *req)
 static void virtio_fs_request_complete(struct fuse_req *req,
 				       struct virtio_fs_vq *fsvq)
 {
-	struct fuse_pqueue *fpq = &fsvq->fud->pq;
 	struct fuse_args *args;
 	struct fuse_args_pages *ap;
 	unsigned int len, i, thislen;
@@ -586,9 +585,7 @@  static void virtio_fs_request_complete(struct fuse_req *req,
 		}
 	}
 
-	spin_lock(&fpq->lock);
 	clear_bit(FR_SENT, &req->flags);
-	spin_unlock(&fpq->lock);
 
 	fuse_request_end(req);
 	spin_lock(&fsvq->lock);