Re: [PATCH 1/2] fuse: Clear FR_PENDING in request_wait_answer

[Joanne Koong <joannelkoong@xxxxxxxxx>]

On Wed, Mar 19, 2025 at 5:37 AM Bernd Schubert <bschubert@xxxxxxx> wrote:
>
> The request is removed from the list of pending requests,
> directly after follows a __fuse_put_request() which is
> likely going to destruct the request, but that is not
> guaranteed, better if FR_PENDING gets cleared.

I think it is guaranteed that the request will be freed. there's only
one call path for request_wait_answer():
__fuse_simple_request()
    fuse_get_req() -> request is allocated (req refcount is 1)
    __fuse_request_send()
        __fuse_get_request() -> req refcount is 2
        fuse_send_one()
        request_wait_answer()
   fuse_put_request() -> req refcount is dropped

if we hit that "if (test_bit(FR_PENDING, &req->flags))" case and
request_wait_answer() drops the ref, then the request will be freed
(or else it's leaked). imo we don't need this patch.


Thanks,
Joanne

>
> Suggested-by: Jeff Layton <jlayton@xxxxxxxxxx>
> Signed-off-by: Bernd Schubert <bschubert@xxxxxxx>
> ---
>  fs/fuse/dev.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 2c3a4d09e500f98232d5d9412a012235af6bec2e..124a6744e8088474efa014a483dc6d297cf321b7 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -438,6 +438,7 @@ static void request_wait_answer(struct fuse_req *req)
>                 /* Request is not yet in userspace, bail out */
>                 if (test_bit(FR_PENDING, &req->flags)) {
>                         list_del(&req->list);
> +                       clear_bit(FR_PENDING, &req->flags);
>                         spin_unlock(&fiq->lock);
>                         __fuse_put_request(req);
>                         req->out.h.error = -EINTR;
>
> --
> 2.43.0
>