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

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

[Jeff Layton]

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

The main difference between v2 and this one is that this one has the new
macros attempt to freeze before going to sleep. It's possible for the
freezer_do_not_count() call to race with the freezer. The FREEZER_SKIP
flag can be set after the freezer has already attempted to freeze the
task. If so, it may just go to sleep without attempting to freeze.

This set attempts to remedy this by calling try_to_freeze() after
calling freezer_do_not_count() and before sleeping. This should prevent
the above race.

I've also fleshed out the comments a bit to nail down the semantics for
these new functions. I'm quite open to changing the names of the
functions as well, if anyone can suggest better alternatives.

Comments and more tested would be appreciated. My goal is to get this
in for 3.3, hopefully 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 |   40 ++++++++++++++++++++++++++++++++++++++++
 net/sunrpc/sched.c      |    3 ++-
 6 files changed, 51 insertions(+), 6 deletions(-)

-- 
1.7.6.4

[PATCH v3 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

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

[Tejun Heo]

Hello,

On Mon, Nov 28, 2011 at 03:01:27PM -0500, Jeff Layton wrote:
> +#define freezable_schedule()						\
> +({									\
> +	freezer_do_not_count();						\
> +	if (!try_to_freeze())						\
> +		schedule();						\
> +	freezer_count();						\
> +})

I don't think you need try_to_freeze() there when you apply this on
top of pm-freezer branch.  Per-task freezing flag is gone and freezing
condition is completely transient from the POV of each task and the
freezer also doesn't care at all whether a task enters freezer or
becomes unfreezable.

Thanks.

-- 
tejun

[PATCH v3 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 |   19 +++++++++++++++++++
 5 files changed, 28 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..fc68c53 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -154,6 +154,22 @@ 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())						\
+		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 +231,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 v3 0/2] nfs/sunrpc: allow freezing of tasks with NFS calls in flight

[Rafael J. Wysocki]

On Monday, November 28, 2011, Jeff Layton wrote:
> 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
> 
> The main difference between v2 and this one is that this one has the new
> macros attempt to freeze before going to sleep. It's possible for the
> freezer_do_not_count() call to race with the freezer. The FREEZER_SKIP
> flag can be set after the freezer has already attempted to freeze the
> task. If so, it may just go to sleep without attempting to freeze.
> 
> This set attempts to remedy this by calling try_to_freeze() after
> calling freezer_do_not_count() and before sleeping. This should prevent
> the above race.
> 
> I've also fleshed out the comments a bit to nail down the semantics for
> these new functions. I'm quite open to changing the names of the
> functions as well, if anyone can suggest better alternatives.
> 
> Comments and more tested would be appreciated. My goal is to get this
> in for 3.3, hopefully via Rafael's tree.

If no one objects, I'll take these patches to linux-pm/linux-next in the
next couple of days (and then to the pm-freezer branch, if there are no
visible problems with them).

Thanks,
Rafael


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

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

[Tejun Heo]

On Mon, Nov 28, 2011 at 09:22:53PM +0100, Rafael J. Wysocki wrote:
> If no one objects, I'll take these patches to linux-pm/linux-next in the
> next couple of days (and then to the pm-freezer branch, if there are no
> visible problems with them).

Rafael, I think these need to be routed through pm-freezer branch.
Freezer went through a lot of changes and I don't think this will work
reliably without the freezer changes.

Thanks.

-- 
tejun

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

[Rafael J. Wysocki]

On Monday, November 28, 2011, Tejun Heo wrote:
> On Mon, Nov 28, 2011 at 09:22:53PM +0100, Rafael J. Wysocki wrote:
> > If no one objects, I'll take these patches to linux-pm/linux-next in the
> > next couple of days (and then to the pm-freezer branch, if there are no
> > visible problems with them).
> 
> Rafael, I think these need to be routed through pm-freezer branch.
> Freezer went through a lot of changes and I don't think this will work
> reliably without the freezer changes.

The linux-next branch contains pm-freezer (or at least it's supposed to).

The point is to let the patches stay in linux-next for a couple of days
before moving them to the pm-freezer branch to see if there's any fallout (I'm
not going to rebase pm-freezer, so fixing it will require addtional commits).

Thanks,
Rafael

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

[Tejun Heo]

Hello,

On Mon, Nov 28, 2011 at 10:07:06PM +0100, Rafael J. Wysocki wrote:
> The point is to let the patches stay in linux-next for a couple of days
> before moving them to the pm-freezer branch to see if there's any fallout (I'm
> not going to rebase pm-freezer, so fixing it will require addtional commits).

Ah.. thanks for the explanation.  Yeah, I'm planning on pulling
freezer changes into cgroup for cgroup-freezer updates as doing it
separately seems to cause a lot of rather unnecessary conflicts so
keeping the branch stable would definitely be nice.

Thanks.

-- 
tejun