[Devel,RHEL7,COMMIT] ms/NFS/CIFS/SUNRPC: don't allow to freeze execution

Submitted by Konstantin Khorenko on Nov. 7, 2016, 7:38 a.m.

Details

Message ID 201611070738.uA77crVk027581@finist_cl7.x64_64.work.ct
State New
Series "ms/NFS/CIFS/SUNRPC: don't allow to freeze execution"
Headers show

Commit Message

Konstantin Khorenko Nov. 7, 2016, 7:38 a.m.
The commit is pushed to "branch-rh7-3.10.0-493.vz7.25.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-493.el7
------>
commit f59597f6c570398c35dfbc1ec2043fdbe0cabebb
Author: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
Date:   Mon Nov 7 11:38:51 2016 +0400

    ms/NFS/CIFS/SUNRPC: don't allow to freeze execution
    
    Here is a deadlock scheme for 2 processes in one freezer cgroup, which is
    freezing:
    
    CPU 0                                   CPU 1
    --------                                --------
    do_last
    inode_lock(dir->d_inode)
    vfs_create
    nfs_create
    ...
    __rpc_execute
    rpc_wait_bit_killable
    __refrigerator
    				    do_last
    				    inode_lock(dir->d_inode)
    
    So, the problem is that one process takes directory inode mutex, executes
    creation request and goes to refrigerator.
    Another one waits till directory lock is released, remains "thawed" and
    thus
    freezer cgroup state never becomes "FROZEN".
    
    Notes:
    1) Interesting, that this is not a pure deadlock: one can thaw cgroup and
    then
    freeze it again.
    2) The issue was introduced by commit
    d310310cbff18ec385c6ab4d58f33b100192a96a.
    3) This patch is not aimed to fix the issue, but to show the problem root.
    Look like this problem might be applicable to other hunks from the commit,
    mentioned above.
    
    Reverts upstream commit: d310310cbff18ec385c6ab4d58f33b100192a96a
    Upstream discussion: https://lkml.org/lkml/2016/8/3/317
    
    https://jira.sw.ru/browse/PSBM-50671
    https://jira.sw.ru/browse/PSBM-54822
    
    Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
---
 fs/cifs/inode.c         |  3 +--
 fs/nfs/inode.c          |  3 +--
 fs/nfs/nfs3proc.c       |  3 +--
 fs/nfs/nfs4proc.c       |  5 ++---
 fs/nfs/proc.c           |  1 -
 include/linux/freezer.h | 23 -----------------------
 net/sunrpc/sched.c      |  2 +-
 7 files changed, 6 insertions(+), 34 deletions(-)

Patch hide | download patch | download mbox

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 3aba214..0b64a52 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -22,7 +22,6 @@ 
 #include <linux/stat.h>
 #include <linux/slab.h>
 #include <linux/pagemap.h>
-#include <linux/freezer.h>
 #include <asm/div64.h>
 #include "cifsfs.h"
 #include "cifspdu.h"
@@ -1856,7 +1855,7 @@  cifs_invalidate_mapping(struct inode *inode)
 static int
 cifs_wait_bit_killable(struct wait_bit_key *key, int mode)
 {
-	freezable_schedule_unsafe();
+	schedule();
 	if (signal_pending_state(mode, current))
 		return -ERESTARTSYS;
 	return 0;
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c
index e432dd4..68fd5ac 100644
--- a/fs/nfs/inode.c
+++ b/fs/nfs/inode.c
@@ -37,7 +37,6 @@ 
 #include <linux/nfs_xdr.h>
 #include <linux/slab.h>
 #include <linux/compat.h>
-#include <linux/freezer.h>
 
 #include <asm/uaccess.h>
 
@@ -77,7 +76,7 @@  nfs_fattr_to_ino_t(struct nfs_fattr *fattr)
  */
 int nfs_wait_bit_killable(struct wait_bit_key *key, int mode)
 {
-	freezable_schedule_unsafe();
+	schedule();
 	if (signal_pending_state(mode, current))
 		return -ERESTARTSYS;
 	return 0;
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index 97a3422..8a72f2d 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -17,7 +17,6 @@ 
 #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"
@@ -34,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)
 			break;
-		freezable_schedule_timeout_killable_unsafe(NFS_JUKEBOX_RETRY_TIME);
+		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 3086873..dbf920e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -54,7 +54,6 @@ 
 #include <linux/module.h>
 #include <linux/xattr.h>
 #include <linux/utsname.h>
-#include <linux/freezer.h>
 
 #include "nfs4_fs.h"
 #include "delegation.h"
@@ -334,7 +333,7 @@  static int nfs4_delay(struct rpc_clnt *clnt, long *timeout)
 
 	might_sleep();
 
-	freezable_schedule_timeout_killable_unsafe(
+	schedule_timeout_killable(
 		nfs4_update_delay(timeout));
 	if (fatal_signal_pending(current))
 		res = -ERESTARTSYS;
@@ -5465,7 +5464,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)
 {
-	freezable_schedule_timeout_killable_unsafe(timeout);
+	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 c63189a..2ddb9a5 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -41,7 +41,6 @@ 
 #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
diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 7fd81b8..d24239c 100644
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -172,14 +172,6 @@  static inline void freezable_schedule(void)
 	freezer_count();
 }
 
-/* DO NOT ADD ANY NEW CALLERS OF THIS FUNCTION */
-static inline void freezable_schedule_unsafe(void)
-{
-	freezer_do_not_count();
-	schedule();
-	freezer_count_unsafe();
-}
-
 /*
  * Like freezable_schedule_timeout(), but should not block the freezer.  Do not
  * call this with locks held.
@@ -216,16 +208,6 @@  static inline long freezable_schedule_timeout_killable(long timeout)
 	return __retval;
 }
 
-/* DO NOT ADD ANY NEW CALLERS OF THIS FUNCTION */
-static inline long freezable_schedule_timeout_killable_unsafe(long timeout)
-{
-	long __retval;
-	freezer_do_not_count();
-	__retval = schedule_timeout_killable(timeout);
-	freezer_count_unsafe();
-	return __retval;
-}
-
 /*
  * Like schedule_hrtimeout_range(), but should not block the freezer.  Do not
  * call this with locks held.
@@ -315,8 +297,6 @@  static inline void set_freezable(void) {}
 
 #define freezable_schedule()  schedule()
 
-#define freezable_schedule_unsafe()  schedule()
-
 #define freezable_schedule_timeout(timeout)  schedule_timeout(timeout)
 
 #define freezable_schedule_timeout_interruptible(timeout)		\
@@ -325,9 +305,6 @@  static inline void set_freezable(void) {}
 #define freezable_schedule_timeout_killable(timeout)			\
 	schedule_timeout_killable(timeout)
 
-#define freezable_schedule_timeout_killable_unsafe(timeout)		\
-	schedule_timeout_killable(timeout)
-
 #define freezable_schedule_hrtimeout_range(expires, delta, mode)	\
 	schedule_hrtimeout_range(expires, delta, mode)
 
diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
index 73ad57a..16ada61 100644
--- a/net/sunrpc/sched.c
+++ b/net/sunrpc/sched.c
@@ -252,7 +252,7 @@  EXPORT_SYMBOL_GPL(rpc_destroy_wait_queue);
 
 static int rpc_wait_bit_killable(struct wait_bit_key *key, int mode)
 {
-	freezable_schedule_unsafe();
+	schedule();
 	if (signal_pending_state(mode, current))
 		return -ERESTARTSYS;
 	return 0;