2011-11-29 12:37:04

by Jeff Layton

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

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



2011-11-29 12:37:11

by Jeff Layton

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

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 <[email protected]>
---
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


2011-11-30 19:40:21

by Rafael J. Wysocki

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

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(-)
>
>


2011-11-29 12:37:07

by Jeff Layton

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

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 <[email protected]>
---
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


2011-12-01 19:32:43

by Adam Williamson

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

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


2011-12-01 18:49:41

by Tejun Heo

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

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

2011-12-01 21:11:00

by Rafael J. Wysocki

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

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