* [PATCH 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight
@ 2011-11-21 17:40 Jeff Layton
2011-11-21 17:40 ` [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events Jeff Layton
2011-11-21 17:40 ` [PATCH 2/2] nfs: make TASK_KILLABLE sleeps attempt to freeze Jeff Layton
0 siblings, 2 replies; 9+ messages in thread
From: Jeff Layton @ 2011-11-21 17:40 UTC (permalink / raw)
To: rjw; +Cc: linux-nfs, linux-pm, john, tj, trond.myklebust
This patchset is a second (third?) attempt at fixing the issues with
suspending a machine that has an active NFS mount.
The bug reported against Fedora is here:
https://bugzilla.redhat.com/show_bug.cgi?id=717735
This patchset is a bit different from the original one. Instead of
attempting to make TASK_KILLABLE sleeps wake up when the freezer runs,
this one makes the freezer skip those processes when it runs.
This is the method that Tejun, Rafael, and Oleg have recommended for
now. Long term, it might make more sense to add a new TASK_WAKEFREEZE
flag or something and awake tasks based on that, and have them then
call try_to_freeze() when awoken. For now though this seems to work
as expected.
So far, results with this patchset have been good, but it seems like
it's a bit late for 3.2. I'd like to see this go in early for 3.3,
probably via Rafael's tree.
Jeff Layton (2):
sunrpc: make rpc_wait_bit_killable handle freeze events
nfs: make TASK_KILLABLE sleeps attempt to freeze
fs/nfs/inode.c | 3 ++-
fs/nfs/nfs3proc.c | 3 ++-
fs/nfs/nfs4proc.c | 5 +++--
fs/nfs/proc.c | 3 ++-
include/linux/freezer.h | 22 ++++++++++++++++++++++
net/sunrpc/sched.c | 3 ++-
6 files changed, 33 insertions(+), 6 deletions(-)
--
1.7.6.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events 2011-11-21 17:40 [PATCH 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight Jeff Layton @ 2011-11-21 17:40 ` Jeff Layton 2011-11-21 17:56 ` Tejun Heo 2011-11-21 20:46 ` Tejun Heo 2011-11-21 17:40 ` [PATCH 2/2] nfs: make TASK_KILLABLE sleeps attempt to freeze Jeff Layton 1 sibling, 2 replies; 9+ messages in thread From: Jeff Layton @ 2011-11-21 17:40 UTC (permalink / raw) To: rjw; +Cc: linux-nfs, linux-pm, john, tj, trond.myklebust Allow the freezer to skip wait_on_bit_killable sleeps in the sunrpc layer. This should allow suspend and hibernate events to proceed, even when there are RPC's pending on the wire. Signed-off-by: Jeff Layton <jlayton@redhat.com> --- include/linux/freezer.h | 12 ++++++++++++ net/sunrpc/sched.c | 3 ++- 2 files changed, 14 insertions(+), 1 deletions(-) diff --git a/include/linux/freezer.h b/include/linux/freezer.h index a5386e3..8b60dd0 100644 --- a/include/linux/freezer.h +++ b/include/linux/freezer.h @@ -135,6 +135,16 @@ static inline void set_freezable_with_signal(void) } /* + * Freezer-friendly macro around schedule() in the kernel. + */ +#define freezable_schedule() \ +({ \ + freezer_do_not_count(); \ + schedule(); \ + freezer_count(); \ +}) + +/* * Freezer-friendly wrappers around wait_event_interruptible(), * wait_event_killable() and wait_event_interruptible_timeout(), originally * defined in <linux/wait.h> @@ -194,6 +204,8 @@ static inline int freezer_should_skip(struct task_struct *p) { return 0; } static inline void set_freezable(void) {} static inline void set_freezable_with_signal(void) {} +#define freezable_schedule() schedule() + #define wait_event_freezable(wq, condition) \ wait_event_interruptible(wq, condition) diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c index d12ffa5..5317b93 100644 --- a/net/sunrpc/sched.c +++ b/net/sunrpc/sched.c @@ -18,6 +18,7 @@ #include <linux/smp.h> #include <linux/spinlock.h> #include <linux/mutex.h> +#include <linux/freezer.h> #include <linux/sunrpc/clnt.h> @@ -231,7 +232,7 @@ static int rpc_wait_bit_killable(void *word) { if (fatal_signal_pending(current)) return -ERESTARTSYS; - schedule(); + freezable_schedule(); return 0; } -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events 2011-11-21 17:40 ` [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events Jeff Layton @ 2011-11-21 17:56 ` Tejun Heo 2011-11-21 20:36 ` Jeff Layton 2011-11-21 20:46 ` Tejun Heo 1 sibling, 1 reply; 9+ messages in thread From: Tejun Heo @ 2011-11-21 17:56 UTC (permalink / raw) To: Jeff Layton; +Cc: rjw, linux-nfs, linux-pm, john, trond.myklebust On Mon, Nov 21, 2011 at 12:40:20PM -0500, Jeff Layton wrote: > Allow the freezer to skip wait_on_bit_killable sleeps in the sunrpc > layer. This should allow suspend and hibernate events to proceed, even > when there are RPC's pending on the wire. > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > --- > include/linux/freezer.h | 12 ++++++++++++ > net/sunrpc/sched.c | 3 ++- > 2 files changed, 14 insertions(+), 1 deletions(-) > > diff --git a/include/linux/freezer.h b/include/linux/freezer.h > index a5386e3..8b60dd0 100644 > --- a/include/linux/freezer.h > +++ b/include/linux/freezer.h > @@ -135,6 +135,16 @@ static inline void set_freezable_with_signal(void) > } > > /* > + * Freezer-friendly macro around schedule() in the kernel. > + */ > +#define freezable_schedule() \ > +({ \ > + freezer_do_not_count(); \ > + schedule(); \ > + freezer_count(); \ Don't we want try_to_freeze() here? If the freezer thought the task was safe to skip, it better stay inside freezer until the freezing condition is lifted. Thanks. -- tejun ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events 2011-11-21 17:56 ` Tejun Heo @ 2011-11-21 20:36 ` Jeff Layton 2011-11-21 20:41 ` Tejun Heo 0 siblings, 1 reply; 9+ messages in thread From: Jeff Layton @ 2011-11-21 20:36 UTC (permalink / raw) To: Tejun Heo; +Cc: rjw, linux-nfs, linux-pm, john, trond.myklebust On Mon, 21 Nov 2011 09:56:44 -0800 Tejun Heo <tj@kernel.org> wrote: > On Mon, Nov 21, 2011 at 12:40:20PM -0500, Jeff Layton wrote: > > Allow the freezer to skip wait_on_bit_killable sleeps in the sunrpc > > layer. This should allow suspend and hibernate events to proceed, even > > when there are RPC's pending on the wire. > > > > Signed-off-by: Jeff Layton <jlayton@redhat.com> > > --- > > include/linux/freezer.h | 12 ++++++++++++ > > net/sunrpc/sched.c | 3 ++- > > 2 files changed, 14 insertions(+), 1 deletions(-) > > > > diff --git a/include/linux/freezer.h b/include/linux/freezer.h > > index a5386e3..8b60dd0 100644 > > --- a/include/linux/freezer.h > > +++ b/include/linux/freezer.h > > @@ -135,6 +135,16 @@ static inline void set_freezable_with_signal(void) > > } > > > > /* > > + * Freezer-friendly macro around schedule() in the kernel. > > + */ > > +#define freezable_schedule() \ > > +({ \ > > + freezer_do_not_count(); \ > > + schedule(); \ > > + freezer_count(); \ > > Don't we want try_to_freeze() here? If the freezer thought the task > was safe to skip, it better stay inside freezer until the freezing > condition is lifted. > > Thanks. > I suppose you're suggesting something like this? freezer_do_not_count(); if (!try_to_freeze()) schedule(); freezer_count(); Hmm, so I guess the concern is that we'd race in such a way that freezer_count() ends up running after the freezer has already skipped the task? If so, that seems just as racy...nothing guarantees that the freeze event occurs before the try_to_freeze call... The freezer is not really my forte', but I have to say that the whole do_not_count scheme seems "sketchy". Would we not be better off with something closer to the original method that I proposed, along with a new TASK_WAKEFREEZE state bit? Processes that are sleeping in uninterruptible sleep that are able to deal with freezer events could set that bit and fake_signal_wake_up could be changed to also wake processes with that bit set. -- Jeff Layton <jlayton@redhat.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events 2011-11-21 20:36 ` Jeff Layton @ 2011-11-21 20:41 ` Tejun Heo 2011-11-21 20:42 ` Tejun Heo 0 siblings, 1 reply; 9+ messages in thread From: Tejun Heo @ 2011-11-21 20:41 UTC (permalink / raw) To: Jeff Layton; +Cc: rjw, linux-nfs, linux-pm, john, trond.myklebust Hello, Jeff. On Mon, Nov 21, 2011 at 03:36:15PM -0500, Jeff Layton wrote: > I suppose you're suggesting something like this? > > freezer_do_not_count(); > if (!try_to_freeze()) > schedule(); > freezer_count(); No, freezer_do_not_count(); schedule(); freezer_count(); try_to_freeze(); freezer_count() may make %current eligible for freezing again, so it has to check whether freezing condition is pending afterwards. > The freezer is not really my forte', but I have to say that the whole > do_not_count scheme seems "sketchy". Would we not be better off with > something closer to the original method that I proposed, along with a > new TASK_WAKEFREEZE state bit? Processes that are sleeping in > uninterruptible sleep that are able to deal with freezer events could > set that bit and fake_signal_wake_up could be changed to also wake > processes with that bit set. AFAICS, the above should be race-free w/ the pending freezer update. http://git.kernel.org/?p=linux/kernel/git/tj/misc.git;a=shortlog;h=refs/heads/pm-freezer Thank you. -- tejun ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events 2011-11-21 20:41 ` Tejun Heo @ 2011-11-21 20:42 ` Tejun Heo 0 siblings, 0 replies; 9+ messages in thread From: Tejun Heo @ 2011-11-21 20:42 UTC (permalink / raw) To: Jeff Layton; +Cc: rjw, linux-nfs, linux-pm, john, trond.myklebust On Mon, Nov 21, 2011 at 12:41:41PM -0800, Tejun Heo wrote: > Hello, Jeff. > > On Mon, Nov 21, 2011 at 03:36:15PM -0500, Jeff Layton wrote: > > I suppose you're suggesting something like this? > > > > freezer_do_not_count(); > > if (!try_to_freeze()) > > schedule(); > > freezer_count(); > > No, > > freezer_do_not_count(); > schedule(); > freezer_count(); > try_to_freeze(); > > freezer_count() may make %current eligible for freezing again, so it > has to check whether freezing condition is pending afterwards. Ooh, freezer_count() already has that. Heh, call me an idiot. The proposed code seems correct then. Thanks. -- tejun ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events 2011-11-21 17:40 ` [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events Jeff Layton 2011-11-21 17:56 ` Tejun Heo @ 2011-11-21 20:46 ` Tejun Heo 2011-11-21 20:57 ` Jeff Layton 1 sibling, 1 reply; 9+ messages in thread From: Tejun Heo @ 2011-11-21 20:46 UTC (permalink / raw) To: Jeff Layton; +Cc: rjw, linux-nfs, linux-pm, john, trond.myklebust On Mon, Nov 21, 2011 at 12:40:20PM -0500, Jeff Layton wrote: > /* > + * Freezer-friendly macro around schedule() in the kernel. > + */ > +#define freezable_schedule() \ > +({ \ > + freezer_do_not_count(); \ > + schedule(); \ > + freezer_count(); \ > +}) So, yes, this seems correct to me but I'm not really sure about the naming. Given what freezable means for other interfaces, this is a bit confusing to me but that could be because I know how each is implemented. Anyone with a better name? Also, this definitely can use a lot more detailed comment. What it's supposed to do, how it's supposed to be used and so on. Thank you. -- tejun ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events 2011-11-21 20:46 ` Tejun Heo @ 2011-11-21 20:57 ` Jeff Layton 0 siblings, 0 replies; 9+ messages in thread From: Jeff Layton @ 2011-11-21 20:57 UTC (permalink / raw) To: Tejun Heo; +Cc: rjw, linux-nfs, linux-pm, john, trond.myklebust On Mon, 21 Nov 2011 12:46:20 -0800 Tejun Heo <tj@kernel.org> wrote: > On Mon, Nov 21, 2011 at 12:40:20PM -0500, Jeff Layton wrote: > > /* > > + * Freezer-friendly macro around schedule() in the kernel. > > + */ > > +#define freezable_schedule() \ > > +({ \ > > + freezer_do_not_count(); \ > > + schedule(); \ > > + freezer_count(); \ > > +}) > > So, yes, this seems correct to me but I'm not really sure about the > naming. Given what freezable means for other interfaces, this is a > bit confusing to me but that could be because I know how each is > implemented. Anyone with a better name? > > Also, this definitely can use a lot more detailed comment. What it's > supposed to do, how it's supposed to be used and so on. > > Thank you. > That makes sense. I'll plan to flesh out the comments in a respin. Yeah, naming these things is rather difficult -- too many "*able" adjectives. I'm quite open to better suggestions for the name (and for wait_event_freezekillable too). -- Jeff Layton <jlayton@redhat.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] nfs: make TASK_KILLABLE sleeps attempt to freeze 2011-11-21 17:40 [PATCH 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight Jeff Layton 2011-11-21 17:40 ` [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events Jeff Layton @ 2011-11-21 17:40 ` Jeff Layton 1 sibling, 0 replies; 9+ messages in thread From: Jeff Layton @ 2011-11-21 17:40 UTC (permalink / raw) To: rjw; +Cc: linux-nfs, linux-pm, john, tj, trond.myklebust Wrap the TASK_KILLABLE sleeps in NFS layer in freezer_do_not_count and freezer_count calls. This allows the freezer to skip these processes when they are sleeping while looping on EJUKEBOX or NFS4ERR_DELAY sorts of errors. Signed-off-by: Jeff Layton <jlayton@redhat.com> --- fs/nfs/inode.c | 3 ++- fs/nfs/nfs3proc.c | 3 ++- fs/nfs/nfs4proc.c | 5 +++-- fs/nfs/proc.c | 3 ++- include/linux/freezer.h | 12 +++++++++++- 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index c07a55a..f36543a 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -38,6 +38,7 @@ #include <linux/nfs_xdr.h> #include <linux/slab.h> #include <linux/compat.h> +#include <linux/freezer.h> #include <asm/system.h> #include <asm/uaccess.h> @@ -77,7 +78,7 @@ int nfs_wait_bit_killable(void *word) { if (fatal_signal_pending(current)) return -ERESTARTSYS; - schedule(); + freezable_schedule(); return 0; } diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c index 85f1690..cc1d1e1 100644 --- a/fs/nfs/nfs3proc.c +++ b/fs/nfs/nfs3proc.c @@ -17,6 +17,7 @@ #include <linux/nfs_page.h> #include <linux/lockd/bind.h> #include <linux/nfs_mount.h> +#include <linux/freezer.h> #include "iostat.h" #include "internal.h" @@ -32,7 +33,7 @@ nfs3_rpc_wrapper(struct rpc_clnt *clnt, struct rpc_message *msg, int flags) res = rpc_call_sync(clnt, msg, flags); if (res != -EJUKEBOX && res != -EKEYEXPIRED) break; - schedule_timeout_killable(NFS_JUKEBOX_RETRY_TIME); + freezable_schedule_timeout_killable(NFS_JUKEBOX_RETRY_TIME); res = -ERESTARTSYS; } while (!fatal_signal_pending(current)); return res; diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index b60fddf..988a0e1 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -53,6 +53,7 @@ #include <linux/sunrpc/bc_xprt.h> #include <linux/xattr.h> #include <linux/utsname.h> +#include <linux/freezer.h> #include "nfs4_fs.h" #include "delegation.h" @@ -241,7 +242,7 @@ static int nfs4_delay(struct rpc_clnt *clnt, long *timeout) *timeout = NFS4_POLL_RETRY_MIN; if (*timeout > NFS4_POLL_RETRY_MAX) *timeout = NFS4_POLL_RETRY_MAX; - schedule_timeout_killable(*timeout); + freezable_schedule_timeout_killable(*timeout); if (fatal_signal_pending(current)) res = -ERESTARTSYS; *timeout <<= 1; @@ -3951,7 +3952,7 @@ int nfs4_proc_delegreturn(struct inode *inode, struct rpc_cred *cred, const nfs4 static unsigned long nfs4_set_lock_task_retry(unsigned long timeout) { - schedule_timeout_killable(timeout); + freezable_schedule_timeout_killable(timeout); timeout <<= 1; if (timeout > NFS4_LOCK_MAXTIMEOUT) return NFS4_LOCK_MAXTIMEOUT; diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c index ac40b85..4dda8b5 100644 --- a/fs/nfs/proc.c +++ b/fs/nfs/proc.c @@ -41,6 +41,7 @@ #include <linux/nfs_fs.h> #include <linux/nfs_page.h> #include <linux/lockd/bind.h> +#include <linux/freezer.h> #include "internal.h" #define NFSDBG_FACILITY NFSDBG_PROC @@ -59,7 +60,7 @@ nfs_rpc_wrapper(struct rpc_clnt *clnt, struct rpc_message *msg, int flags) res = rpc_call_sync(clnt, msg, flags); if (res != -EKEYEXPIRED) break; - schedule_timeout_killable(NFS_JUKEBOX_RETRY_TIME); + freezable_schedule_timeout_killable(NFS_JUKEBOX_RETRY_TIME); res = -ERESTARTSYS; } while (!fatal_signal_pending(current)); return res; diff --git a/include/linux/freezer.h b/include/linux/freezer.h index 8b60dd0..235cb05 100644 --- a/include/linux/freezer.h +++ b/include/linux/freezer.h @@ -135,7 +135,7 @@ static inline void set_freezable_with_signal(void) } /* - * Freezer-friendly macro around schedule() in the kernel. + * Freezer-friendly macros around some sleeping functions. */ #define freezable_schedule() \ ({ \ @@ -144,6 +144,13 @@ static inline void set_freezable_with_signal(void) freezer_count(); \ }) +#define freezable_schedule_timeout_killable(timeout) \ +({ \ + freezer_do_not_count(); \ + schedule_timeout_killable(timeout); \ + freezer_count(); \ +}) + /* * Freezer-friendly wrappers around wait_event_interruptible(), * wait_event_killable() and wait_event_interruptible_timeout(), originally @@ -206,6 +213,9 @@ static inline void set_freezable_with_signal(void) {} #define freezable_schedule() schedule() +#define freezable_schedule_timeout_killable(timeout) \ + schedule_timeout_killable(timeout) + #define wait_event_freezable(wq, condition) \ wait_event_interruptible(wq, condition) -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-11-21 20:58 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-21 17:40 [PATCH 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight Jeff Layton 2011-11-21 17:40 ` [PATCH 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events Jeff Layton 2011-11-21 17:56 ` Tejun Heo 2011-11-21 20:36 ` Jeff Layton 2011-11-21 20:41 ` Tejun Heo 2011-11-21 20:42 ` Tejun Heo 2011-11-21 20:46 ` Tejun Heo 2011-11-21 20:57 ` Jeff Layton 2011-11-21 17:40 ` [PATCH 2/2] nfs: make TASK_KILLABLE sleeps attempt to freeze Jeff Layton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).