[PATCH v4 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight

[PATCH v4 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight

[Jeff Layton]

This patchset is the fourth 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

The main difference from v3 is that I've fixed a stupid set of errors in
freezable_schedule_timeout_killable. The if condition was reversed from
what it should have been, and the code did not call freezer_count()
again when it was going to return early.

Tejun has mentioned that the extra try_to_freeze() calls are unnecessary
on top of the pm-freezer changes, but this set should work on as
expected on top of 3.2-ish kernels. In the meantime, I'll look over the
pm-freezer changes and see whether I need to make changes in this set
for 3.3.

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 |   42 ++++++++++++++++++++++++++++++++++++++++++
 net/sunrpc/sched.c      |    3 ++-
 6 files changed, 53 insertions(+), 6 deletions(-)

-- 
1.7.6.4

[PATCH v4 1/2] sunrpc: make rpc_wait_bit_killable handle freeze events

[Jeff Layton]

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 |   21 +++++++++++++++++++++
 net/sunrpc/sched.c      |    3 ++-
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index a5386e3..fda3ac6 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -135,6 +135,25 @@ static inline void set_freezable_with_signal(void)
 }
 
 /*
+ * These macros are intended to be used whenever you want allow a task that's
+ * sleeping in TASK_UNINTERRUPTIBLE or TASK_KILLABLE state to be frozen.
+ *
+ */
+
+/*
+ * Like schedule(), but should not block the freezer. It may return immediately
+ * if it ends up racing with the freezer. Callers must be able to deal with
+ * spurious wakeups.
+ */
+#define freezable_schedule()						\
+({									\
+	freezer_do_not_count();						\
+	if (!try_to_freeze())						\
+		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 +213,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

[PATCH v4 2/2] nfs: make TASK_KILLABLE sleeps attempt to freeze

[Jeff Layton]

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 |   21 +++++++++++++++++++++
 5 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index 50a15fa..bf3a57b 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 d4bc9ed9..9194395 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 be2bbac..b28bb19 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;
@@ -3950,7 +3951,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 f48125d..0c672588 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 fda3ac6..29b5877 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -154,6 +154,24 @@ static inline void set_freezable_with_signal(void)
 })
 
 /*
+ * Like schedule_timeout_killable(), but should not block the freezer. It may
+ * end up returning immediately if it ends up racing with the freezer. Callers
+ * must be able to deal with the loose wakeup timing that can occur when the
+ * freezer races in. When that occurs, this function will return the timeout
+ * value instead of 0.
+ */
+#define freezable_schedule_timeout_killable(timeout)			\
+({									\
+	freezer_do_not_count();						\
+	if (try_to_freeze()) {						\
+		freezer_count();					\
+		return timeout;						\
+	}								\
+	schedule_timeout_killable(timeout);				\
+	freezer_count();						\
+})
+
+/*
  * Freezer-friendly wrappers around wait_event_interruptible(),
  * wait_event_killable() and wait_event_interruptible_timeout(), originally
  * defined in <linux/wait.h>
@@ -215,6 +233,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

Re: [PATCH v4 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight

[Rafael J. Wysocki]

On Tuesday, November 29, 2011, Jeff Layton wrote:
> This patchset is the fourth 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
> 
> The main difference from v3 is that I've fixed a stupid set of errors in
> freezable_schedule_timeout_killable. The if condition was reversed from
> what it should have been, and the code did not call freezer_count()
> again when it was going to return early.
> 
> Tejun has mentioned that the extra try_to_freeze() calls are unnecessary
> on top of the pm-freezer changes, but this set should work on as
> expected on top of 3.2-ish kernels. In the meantime, I'll look over the
> pm-freezer changes and see whether I need to make changes in this set
> for 3.3.
> 
> Jeff Layton (2):
>   sunrpc: make rpc_wait_bit_killable handle freeze events
>   nfs: make TASK_KILLABLE sleeps attempt to freeze

Both patches applied to linux-pm/linux-next.  I'll move them to
linux-pm/pm-freezer in a couple of days if there are no problems with
them.

Thanks,
Rafael


>  fs/nfs/inode.c          |    3 ++-
>  fs/nfs/nfs3proc.c       |    3 ++-
>  fs/nfs/nfs4proc.c       |    5 +++--
>  fs/nfs/proc.c           |    3 ++-
>  include/linux/freezer.h |   42 ++++++++++++++++++++++++++++++++++++++++++
>  net/sunrpc/sched.c      |    3 ++-
>  6 files changed, 53 insertions(+), 6 deletions(-)
> 
> 

Re: [PATCH v4 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight

[Tejun Heo]

On Wed, Nov 30, 2011 at 08:43:13PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, November 29, 2011, Jeff Layton wrote:
> > This patchset is the fourth 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
> > 
> > The main difference from v3 is that I've fixed a stupid set of errors in
> > freezable_schedule_timeout_killable. The if condition was reversed from
> > what it should have been, and the code did not call freezer_count()
> > again when it was going to return early.
> > 
> > Tejun has mentioned that the extra try_to_freeze() calls are unnecessary
> > on top of the pm-freezer changes, but this set should work on as
> > expected on top of 3.2-ish kernels. In the meantime, I'll look over the
> > pm-freezer changes and see whether I need to make changes in this set
> > for 3.3.
> > 
> > Jeff Layton (2):
> >   sunrpc: make rpc_wait_bit_killable handle freeze events
> >   nfs: make TASK_KILLABLE sleeps attempt to freeze
> 
> Both patches applied to linux-pm/linux-next.  I'll move them to
> linux-pm/pm-freezer in a couple of days if there are no problems with
> them.

Rafael, I think the extra try_to_freeze() is actually incorrect on top
of pm-freezer.  Jeff, can you please remove that?

Thanks.

-- 
tejun

Re: [PATCH v4 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight

[Adam Williamson]

On Thu, 2011-12-01 at 10:49 -0800, Tejun Heo wrote:
> On Wed, Nov 30, 2011 at 08:43:13PM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, November 29, 2011, Jeff Layton wrote:
> > > This patchset is the fourth 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
> > > 
> > > The main difference from v3 is that I've fixed a stupid set of errors in
> > > freezable_schedule_timeout_killable. The if condition was reversed from
> > > what it should have been, and the code did not call freezer_count()
> > > again when it was going to return early.
> > > 
> > > Tejun has mentioned that the extra try_to_freeze() calls are unnecessary
> > > on top of the pm-freezer changes, but this set should work on as
> > > expected on top of 3.2-ish kernels. In the meantime, I'll look over the
> > > pm-freezer changes and see whether I need to make changes in this set
> > > for 3.3.
> > > 
> > > Jeff Layton (2):
> > >   sunrpc: make rpc_wait_bit_killable handle freeze events
> > >   nfs: make TASK_KILLABLE sleeps attempt to freeze
> > 
> > Both patches applied to linux-pm/linux-next.  I'll move them to
> > linux-pm/pm-freezer in a couple of days if there are no problems with
> > them.
> 
> Rafael, I think the extra try_to_freeze() is actually incorrect on top
> of pm-freezer.  Jeff, can you please remove that?

You might want to read two levels up: Jeff explicitly mentioned that
already. :)
-- 
Adam Williamson
Fedora QA Community Monkey
IRC: adamw | Twitter: AdamW_Fedora | identi.ca: adamwfedora
http://www.happyassassin.net

Re: [PATCH v4 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight

[Rafael J. Wysocki]

On Thursday, December 01, 2011, Adam Williamson wrote:
> On Thu, 2011-12-01 at 10:49 -0800, Tejun Heo wrote:
> > On Wed, Nov 30, 2011 at 08:43:13PM +0100, Rafael J. Wysocki wrote:
> > > On Tuesday, November 29, 2011, Jeff Layton wrote:
> > > > This patchset is the fourth 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
> > > > 
> > > > The main difference from v3 is that I've fixed a stupid set of errors in
> > > > freezable_schedule_timeout_killable. The if condition was reversed from
> > > > what it should have been, and the code did not call freezer_count()
> > > > again when it was going to return early.
> > > > 
> > > > Tejun has mentioned that the extra try_to_freeze() calls are unnecessary
> > > > on top of the pm-freezer changes, but this set should work on as
> > > > expected on top of 3.2-ish kernels. In the meantime, I'll look over the
> > > > pm-freezer changes and see whether I need to make changes in this set
> > > > for 3.3.
> > > > 
> > > > Jeff Layton (2):
> > > >   sunrpc: make rpc_wait_bit_killable handle freeze events
> > > >   nfs: make TASK_KILLABLE sleeps attempt to freeze
> > > 
> > > Both patches applied to linux-pm/linux-next.  I'll move them to
> > > linux-pm/pm-freezer in a couple of days if there are no problems with
> > > them.
> > 
> > Rafael, I think the extra try_to_freeze() is actually incorrect on top
> > of pm-freezer.  Jeff, can you please remove that?
> 
> You might want to read two levels up: Jeff explicitly mentioned that
> already. :)

Well, that means I put those patches into linux-pm/linux-next too early,
since that branch already includes pm-freezer.  I'll drop them from there
temporarily until Jeff rebases them on top of pm-freezer (or
linux-pm/linux-next).

Thanks,
Rafael