[Devel] [PATCH VZ9] fs: fuse: krpc: fix file leakage in krpc

[Vasily Averin]

Alexey,
I gave no access to sources but batch looks strange for me,
 please  take look at my comment in end of this letter

Thank you,
	Vasily Averin

On 11/12/24 18:12, Alexey Kuznetsov wrote:
> Obvious leak, which cannot detected unless you try to unload
> fuse module. Found accidentally searching for reasons why
> fuse_dev_find_request take 7% of cpu in profiles.
> 
> Also, make it in more optimal way, fget() is too expensive and
> well seen on profiles.
> 
> NOTE: this place also needs rcufication. I did that, it is simple
> but patch is too intrusive and suprizingly does not show too
> much of win. It looks like cache bouncing between krpc thread
> and client thread sending request is dominating rather
> than spinlock contention/bouncing.
> 
> Signed-off-by: Alexey Kuznetsov <kuznet at virtuozzo.com>
> ---
>  fs/fuse/dev.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index f5594e4..5967248 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2678,16 +2678,28 @@ int fuse_dev_release(struct inode *inode, struct file *file)
>  
>  struct fuse_req *fuse_dev_find_request(int fd, u64 unique)
>  {
> -	struct file *f = fget(fd);
> -	struct fuse_dev *fud = fuse_get_dev(f);
> -	struct fuse_pqueue *fpq = &fud->pq;
> +	struct file * file;
> +	struct fuse_dev *fud;
> +	struct fuse_pqueue *fpq;
>  	struct fuse_req *req = NULL;
>  
>  	spin_lock(&fpq->lock);
> +	file = files_lookup_fd_rcu(current->files, fd);
> +	if (!file)
> +		return NULL;
> +
> +	if (file->f_op != &fuse_dev_operations)
> +		return NULL;
> +
> +	fud = fuse_get_dev(file);
> +	fpq = &fud->pq;
> +
>  	if (fpq->connected)
>  		req = request_find(&fud->pq, unique);
> -	spin_unlock(&fpq->lock);
>  
> +	if (req && hlist_unhashed_lockless(&req->hlist))
> +		req = NULL;
> +	spin_lock(&fpq->lock);
   <<<<<< unlock should be here perhaps? before patch function was returned without taken lock..
>  	return req;
>  }
>  EXPORT_SYMBOL_GPL(fuse_dev_find_request);