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

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

[Sergey Senozhatsky]

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 <senozhatsky@chromium.org>
---
 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);
 		if (!err)
 			return;
 
@@ -719,8 +721,9 @@ static void request_wait_answer(struct fuse_req *req)
 		bool removed;
 
 		/* Only fatal signals may interrupt this */
-		err = wait_event_killable(req->waitq,
-					test_bit(FR_FINISHED, &req->flags));
+		err = wait_event_state(req->waitq,
+				       test_bit(FR_FINISHED, &req->flags),
+				       TASK_KILLABLE | TASK_FREEZABLE_UNSAFE);
 		if (!err)
 			return;
 
@@ -741,7 +744,8 @@ static void request_wait_answer(struct fuse_req *req)
 	 * Either request is already in userspace, or it was forced.
 	 * Wait it out.
 	 */
-	wait_event(req->waitq, test_bit(FR_FINISHED, &req->flags));
+	wait_event_state(req->waitq, test_bit(FR_FINISHED, &req->flags),
+			 TASK_UNINTERRUPTIBLE | TASK_FREEZABLE_UNSAFE);
 }
 
 static void __fuse_request_send(struct fuse_req *req)
-- 
2.55.0.679.g6767b8d81c-goog

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

[Sergey Senozhatsky]

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 <senozhatsky@chromium.org>
> ---
>  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.