[PATCH] virtio_fs: Remove redundant spinlock in virtio_fs_request_complete()

[PATCH] virtio_fs: Remove redundant spinlock in virtio_fs_request_complete()

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

Since clear_bit is an atomic operation, the spinlock is redundant and
can be removed, reducing lock contention is good for performance.

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 fs/fuse/virtio_fs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 8f2e2f3..de34179 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -791,9 +791,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);
-- 
2.9.4

Re: [PATCH] virtio_fs: Remove redundant spinlock in virtio_fs_request_complete()

[kernel test robot]

Hi lirongqing,

kernel test robot noticed the following build warnings:

[auto build test WARNING on mszeredi-fuse/for-next]
[also build test WARNING on linus/master v6.16-rc1 next-20250613]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/lirongqing/virtio_fs-Remove-redundant-spinlock-in-virtio_fs_request_complete/20250613-135306
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next
patch link:    https://lore.kernel.org/r/20250613055051.1873-1-lirongqing%40baidu.com
patch subject: [PATCH] virtio_fs: Remove redundant spinlock in virtio_fs_request_complete()
config: i386-randconfig-003-20250614 (https://download.01.org/0day-ci/archive/20250614/202506140329.g0oJMDcD-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250614/202506140329.g0oJMDcD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506140329.g0oJMDcD-lkp@intel.com/

All warnings (new ones prefixed by >>):

   fs/fuse/virtio_fs.c: In function 'virtio_fs_request_complete':
>> fs/fuse/virtio_fs.c:765:29: warning: unused variable 'fpq' [-Wunused-variable]
     765 |         struct fuse_pqueue *fpq = &fsvq->fud->pq;
         |                             ^~~


vim +/fpq +765 fs/fuse/virtio_fs.c

a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  760  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  761  /* Work function for request completion */
bb737bbe48bea9 Vivek Goyal     2020-04-20  762  static void virtio_fs_request_complete(struct fuse_req *req,
bb737bbe48bea9 Vivek Goyal     2020-04-20  763  				       struct virtio_fs_vq *fsvq)
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  764  {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12 @765  	struct fuse_pqueue *fpq = &fsvq->fud->pq;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  766  	struct fuse_args *args;
bb737bbe48bea9 Vivek Goyal     2020-04-20  767  	struct fuse_args_pages *ap;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  768  	unsigned int len, i, thislen;
29279e1d4284a2 Joanne Koong    2024-10-24  769  	struct folio *folio;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  770  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  771  	/*
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  772  	 * TODO verify that server properly follows FUSE protocol
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  773  	 * (oh.uniq, oh.len)
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  774  	 */
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  775  	args = req->args;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  776  	copy_args_from_argbuf(args, req);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  777  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  778  	if (args->out_pages && args->page_zeroing) {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  779  		len = args->out_args[args->out_numargs - 1].size;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  780  		ap = container_of(args, typeof(*ap), args);
29279e1d4284a2 Joanne Koong    2024-10-24  781  		for (i = 0; i < ap->num_folios; i++) {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  782  			thislen = ap->descs[i].length;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  783  			if (len < thislen) {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  784  				WARN_ON(ap->descs[i].offset);
68bfb7eb7f7de3 Joanne Koong    2024-10-24  785  				folio = ap->folios[i];
68bfb7eb7f7de3 Joanne Koong    2024-10-24  786  				folio_zero_segment(folio, len, thislen);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  787  				len = 0;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  788  			} else {
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  789  				len -= thislen;
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  790  			}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  791  		}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  792  	}
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  793  
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  794  	clear_bit(FR_SENT, &req->flags);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  795  
8f622e9497bbbd Max Reitz       2020-04-20  796  	fuse_request_end(req);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  797  	spin_lock(&fsvq->lock);
c17ea009610366 Vivek Goyal     2019-10-15  798  	dec_in_flight_req(fsvq);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  799  	spin_unlock(&fsvq->lock);
a62a8ef9d97da2 Stefan Hajnoczi 2018-06-12  800  }
bb737bbe48bea9 Vivek Goyal     2020-04-20  801  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] virtio_fs: Remove redundant spinlock in virtio_fs_request_complete()

[Stefan Hajnoczi]

On Fri, Jun 13, 2025 at 1:51 AM lirongqing <lirongqing@baidu.com> wrote:
>
> From: Li RongQing <lirongqing@baidu.com>
>
> Since clear_bit is an atomic operation, the spinlock is redundant and
> can be removed, reducing lock contention is good for performance.
>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>  fs/fuse/virtio_fs.c | 2 --
>  1 file changed, 2 deletions(-)

The spin lock originally protected req->list, so the lock has no use here.

The initial req->list access is still protected by fpq->lock in
virtio_fs_requests_done_work():
  while ((req = virtqueue_get_buf(vq, &len)) != NULL) {
      spin_lock(&fpq->lock);
      list_move_tail(&req->list, &reqs);
      spin_unlock(&fpq->lock);
  }

Looks safe, but please see the kernel test robot email about an unused
variable warning.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

>
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index 8f2e2f3..de34179 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -791,9 +791,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);
> --
> 2.9.4
>
>