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

Alexey Kuznetsov kuznet at virtuozzo.com
Tue Nov 12 19:20:19 MSK 2024


Thank you! Thank you! Thank you!

The patch is forged crap, to my shame :-) It is result of manual edit.
I did it in hurry finding out backlogged terminal window and being distracted
by urgent tasks and did not even check it before submission. Shameful.
Not only unlock, it is still from my private tree, it refers to fields
which are still absent in git.

On Tue, Nov 12, 2024 at 11:13 PM Alexey Kuznetsov <kuznet at virtuozzo.com> 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);
>         return req;
>  }
>  EXPORT_SYMBOL_GPL(fuse_dev_find_request);
> --
> 1.8.3.1
>



More information about the Devel mailing list