[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fuse-devel
Subject:    Re: [fuse-devel] [PATCH] fuse: Freeze client on suspend when
From:       Miklos Szeredi <miklos () szeredi ! hu>
Date:       2011-08-25 14:20:07
Message-ID: 87hb55fu94.fsf () tucsk ! pomaz ! szeredi ! hu
[Download RAW message or body]

Todd Poynor <toddpoynor@google.com> 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@google.com>
> ---
> 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)

------------------------------------------------------------------------------
EMC VNX: the world's simplest storage, starting under $10K
The only unified storage solution that offers unified management 
Up to 160% more powerful than alternatives and 25% more efficient. 
Guaranteed. http://p.sf.net/sfu/emc-vnx-dev2dev
_______________________________________________
fuse-devel mailing list
fuse-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/fuse-devel
[prev in list] [next in list] [prev in thread] [next in thread]