[Devel] [PATCH] fs/fuse: fix locked kio request send

[Kirill Tkhai]

On 29.05.2019 12:12, Pavel Butsykin wrote:
> Since patch 'fs/fuse kio: simplify processing and sending kio requests',
> fuse_request_queue_background() function cannot be executed under spinlock,
> because inserting kio request to RPC queue implies additional memory
> allocations, such as pcs_int_request, fuse_req, pcs_ioc_getmap, pcs_map_entry,
> etc.
> 
> We can release fi->lock inside fuse_send_writepage() for the duration of
> execution fuse_request_queue_background(), because in this place fi->lock
> protects only fi->writectr.
> 
> Call trace:
> [95042.681286]  [<ffffffff87a7fbdf>] dump_stack+0x19/0x1b
> [95042.681289]  [<ffffffff872f0723>] __might_sleep+0x173/0x230
> [95042.681293]  [<ffffffff8748f463>] kmem_cache_alloc+0x2d3/0x430
> [95042.681326]  [<ffffffffc0a40a10>] ireq_process+0x26/0x28 [fuse_kio_pcs]
> [95042.681329]  [<ffffffffc0a449fb>] pcs_fuse_submit+0x4df/0x4f5 [fuse_kio_pcs]
> [95042.681332]  [<ffffffffc0a456cd>] kpcs_req_send+0x1b2/0x35d [fuse_kio_pcs]
> [95042.681334]  [<ffffffff872ffca5>] ? local_clock+0x25/0x30
> [95042.681338]  [<ffffffff8734bdcf>] ? lock_release_holdtime.part.30+0xf/0x1a0
> [95042.681342]  [<ffffffff8764387b>] ? do_raw_spin_unlock+0x4b/0x90
> [95042.681345]  [<ffffffffc0962187>] flush_bg_queue+0x1c6/0x216 [fuse]
> [95042.681348]  [<ffffffffc0963510>] fuse_request_queue_background+0x358/0x374 [fuse]
> [95042.681353]  [<ffffffffc097562b>] fuse_send_writepage+0xd9/0x165 [fuse]
> [95042.681356]  [<ffffffffc0975720>] fuse_flush_writepages+0x69/0x8f [fuse]
> [95042.681359]  [<ffffffffc09763dc>] fuse_send_writepages+0x582/0x5e3 [fuse]
> [95042.681362]  [<ffffffffc096f117>] ? get_fuse_conn+0x1c/0x1e [fuse]
> 
> Signed-off-by: Pavel Butsykin <pbutsykin at virtuozzo.com>

Reviewed-by: Kirill Tkhai <ktkhai at virtuozzo.com>

> ---
>  fs/fuse/file.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index a1dc3216da87..259eef0fa005 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -1950,6 +1950,8 @@ __acquires(fi->lock)
>  	__u64 data_size = req->num_pages * PAGE_CACHE_SIZE;
>  	bool queued;
>  
> +	fi->writectr++;
> +
>  	if (inarg->offset + data_size <= size) {
>  		inarg->size = data_size;
>  	} else if (inarg->offset < size) {
> @@ -1960,15 +1962,18 @@ __acquires(fi->lock)
>  	}
>  
>  	req->in.args[1].size = inarg->size;
> +	spin_unlock(&fi->lock);
> +
>  	queued = fuse_request_queue_background(fc, req);
> +	spin_lock(&fi->lock);
>  	/* Fails on broken connection only */
>  	if (unlikely(!queued))
>  		goto out_free;
>  
> -	fi->writectr++;
>  	return;
>  
>   out_free:
> + 	fi->writectr--;
>  	fuse_writepage_finish(fc, req);
>  	spin_unlock(&fi->lock);
>  	fuse_writepage_free(fc, req);
>