Re: [PATCH] fuse: Freeze client on suspend when request sent to userspace

[Miklos Szeredi]

Todd Poynor <toddpoynor@xxxxxxxxxx> writes:

> Suspend attempts can abort when the FUSE daemon is already frozen
> and a client is waiting uninterruptibly for a response, causing
> freezing of tasks to fail.
>
> Use the freeze-friendly wait API, but disregard other signals.
>
> Signed-off-by: Todd Poynor <toddpoynor@xxxxxxxxxx>
> ---
> Have seen reports in which repeated suspend attempts were aborted
> due to a task waiting uninterruptibly in this function, but
> have only reproduced this artificially, by causing the daemon to
> sleep.  Only modified the normal request path, not request aborts
> and such, under the assumption that these should be rare and
> should make progress upon resume.  Certain apps that read or
> write a lot of data on the filesystem may apparently run into
> this case rather frequently.


Yes, this is a well known problem with no (known) trivial solutions.

Problem with the patch is, it's going to solve only a subset of the
suspend aborts.  It will allow any operation sent to userspace to
freeze, which is fine.  But imagine that such a request is holding a VFS
lock (e.g. write(2) is hoding i_mutex) and another operation is waiting
on that lock.  That one still won't be freezable and will prevent
suspend to proceed.

I'm not sure it's really worth fixing only a subset of the problematic
cases.  We should definitely be thinking about solving it properly, in
all cases.

Thanks,
Miklos

>
>  fs/fuse/dev.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 168a80f..bded2e5 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -19,6 +19,7 @@
>  #include <linux/pipe_fs_i.h>
>  #include <linux/swap.h>
>  #include <linux/splice.h>
> +#include <linux/freezer.h>
>  
>  MODULE_ALIAS_MISCDEV(FUSE_MINOR);
>  MODULE_ALIAS("devname:fuse");
> @@ -383,7 +384,10 @@ __acquires(fc->lock)
>  	 * Wait it out.
>  	 */
>  	spin_unlock(&fc->lock);
> -	wait_event(req->waitq, req->state == FUSE_REQ_FINISHED);
> +
> +	while (req->state != FUSE_REQ_FINISHED)
> +		wait_event_freezable(req->waitq,
> +				     req->state == FUSE_REQ_FINISHED);
>  	spin_lock(&fc->lock);
>  
>  	if (!req->aborted)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/