[<prev] [next>] [day] [month] [year] [list]
Message-ID: <87hb55fu94.fsf@tucsk.pomaz.szeredi.hu>
Date: Thu, 25 Aug 2011 16:20:07 +0200
From: Miklos Szeredi <miklos@...redi.hu>
To: Todd Poynor <toddpoynor@...gle.com>
Cc: fuse-devel@...ts.sourceforge.net,
Linux PM mailing list <linux-pm@...ts.linux-foundation.org>,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fuse: Freeze client on suspend when request sent to userspace
Todd Poynor <toddpoynor@...gle.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@...gle.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)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists