Re: [RFC PATCH] fuse: permit freezing while waiting for request answer

On (26/08/12 18:54), Sergey Senozhatsky wrote:
> Suspend freezes tasks in random order and doesn't take into
> consideration producer-consumer dependency that may exist
> between tasks.  One example where this can cause issues is:
> fuse server getting frozen ahead of clients, which then get
> stuck waiting for req answers that never come (the server
> is already frozen):
> 
>  PM: suspend entry (s2idle)
>  Filesystems sync: 0.018 seconds
>  Freezing user space processes
>  Freezing user space processes failed after 20.001 seconds (1 tasks refusing to freeze, wq_busy=0):
>  task:ThreadPoolForeg state:D stack:0     pid:7873  tgid:7854  ppid:7827   flags:0x00004006
>  Call Trace:
>   <TASK>
>   __schedule+0x554/0x1320
>   ? vprintk_emit+0x2a8/0x320
>   schedule+0x5e/0xd0
>   __fuse_simple_request+0x4c8/0x6d0
>   fuse_do_getattr+0x1e9/0x320
>   fuse_update_get_attr+0x362/0x6a0
>   fuse_file_read_iter+0x13a/0x1b0
>   vfs_read+0x29f/0x2f0
>   ksys_read+0x75/0xf0
>   do_syscall_64+0x70/0xf0
> 
> Make all wait-event calls in request_wait_answer() freezer-friendly.
> This uses TASK_FREEZABLE_UNSAFE because often time we freeze holding
> locks (in the upper layers), which triggers debug_locks warning.
> 
> Signed-off-by: Sergey Senozhatsky <[email protected]>
> ---
>  fs/fuse/dev.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 27dafda2a841..9e98ece4143f 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -703,8 +703,10 @@ static void request_wait_answer(struct fuse_req *req)
>  
>  	if (!fch->no_interrupt) {
>  		/* Any signal may interrupt this */
> -		err = wait_event_interruptible(req->waitq,
> -					test_bit(FR_FINISHED, &req->flags));
> +		err = wait_event_state(req->waitq,
> +				       test_bit(FR_FINISHED, &req->flags),
> +				       TASK_INTERRUPTIBLE |
> +				       TASK_FREEZABLE_UNSAFE);

Or I can switch to TASK_FREEZABLE, people probably don't run fuse
under lockdep, and that debug locks warning is WARN_ONCE() and
is not a terminal condition for lockdep.  We landed TASK_FREEZABLE
a while back in fuse_get_req().  Alternatively, I can also switch
fuse_get_req() to TASK_FREEZABLE_UNSAFE.  It sort of exists for
cases like this, on one hand; I understand that adding new
FREEZABLE_UNSAFE users might be problematic on the other hand.

We use fuse on consumer devices (laptops) and suspend is important,
failing suspend is a bad user experience.