[Devel,libvzctl] fast stop: suppress SUNRPC per task

Submitted by Stanislav Kinsburskiy on Aug. 21, 2017, 1:18 p.m.

Details

Message ID 20170821131820.68192.43251.stgit@skinsbursky-vz7.qa.sw.ru
State New
Series "fast stop: suppress SUNRPC per task"
Headers show

Commit Message

Stanislav Kinsburskiy Aug. 21, 2017, 1:18 p.m.
It solves two issues.
First, it allows to kill tasks in nested namespaces (rare, but possible case).
Second, is allows to kill CT with exited child reaper (when it's almost dead,
but waiting for it's children), because in this case SUNRPC can't be supressed
via child reapers proc dentries (/proc/pid/net is not available anymore).

https://jira.sw.ru/browse/PSBM-70437

Note: this patch reverts (replaces) commit
f8e953f5d683aa6f04dcc2b5634444b98d6e9ee3

Signed-off-by: Stanislav Kinsburskiy <skinsbursky@virtuozzo.com>
---
 lib/env_nsops.c |   81 +++++++++++++++++++++++++------------------------------
 1 file changed, 37 insertions(+), 44 deletions(-)

Patch hide | download patch | download mbox

diff --git a/lib/env_nsops.c b/lib/env_nsops.c
index e5b662d..a565c69 100644
--- a/lib/env_nsops.c
+++ b/lib/env_nsops.c
@@ -1058,6 +1058,36 @@  static int ns_env_exec_fn(struct vzctl_env_handle *h, execFn fn, void *data,
 	return 0;
 }
 
+static int write_sunrpc_kill(pid_t pid, unsigned value)
+{
+	int fd;
+	ssize_t res;
+	char path[PATH_MAX];
+	char *val = value ? "1" : "0";

+	snprintf(path, sizeof(path), "/proc/%d/net/rpc/kill-tasks", pid);
+
+	if (access(path, F_OK))
+		return 0;
+
+	fd = open(path, O_WRONLY);
+	if (fd == -1) {
+		if (errno == ENOENT)
+			return 0;
+		return vzctl_err(-1, errno, "Failed to open %s: %s",
+				path, strerror(errno));
+	}
+
+	res = write(fd, val, strlen(val) + 1);
+	close(fd);
+
+	if (res != strlen(val) + 1)
+		return vzctl_err(-1, errno, "Unable to %s SUNRPC traffic",
+				value ? "suppress" : "release");
+
+	return 1;
+}
+
 static int ns_env_kill(struct vzctl_env_handle *h)
 {
 	int ret;
@@ -1076,6 +1106,8 @@  static int ns_env_kill(struct vzctl_env_handle *h)
 			continue;
 		}
 
+		write_sunrpc_kill(pid, 1);
+
 		logger(5, 0, "kill CT process pid %lu", pid);
 		if (kill(pid, SIGKILL) && errno != ESRCH)
 			vzctl_err(-1, errno, "Failed to kill CT pid=%lu", pid);
@@ -1086,70 +1118,31 @@  static int ns_env_kill(struct vzctl_env_handle *h)
 	return 0;
 }
 
-static int write_sunrpc_kill(struct vzctl_env_handle *h, unsigned value)
-{
-	pid_t pid;
-	int fd;
-	ssize_t res;
-	char path[PATH_MAX];
-	char *val = value ? "1" : "0";
-
-	if (cg_env_get_init_pid(h->ctid, &pid))
-		return -1;
-
-	snprintf(path, sizeof(path), "/proc/%d/net/rpc/kill-tasks", pid);
-
-	if (access(path, F_OK))
-		return 0;
-
-	fd = open(path, O_WRONLY);
-	if (fd == -1)
-		return vzctl_err(-1, errno, "Failed to open %s: %s",
-				path, strerror(errno));
-
-	res = write(fd, val, strlen(val) + 1);
-	close(fd);
-
-	if (res != strlen(val) + 1)
-		return vzctl_err(-1, errno, "Unable to %s SUNRPC traffic",
-				value ? "suppress" : "release");
-	return 1;
-}
-
 static int ns_env_stop_force(struct vzctl_env_handle *h)
 {
-	int ret, rc, sunrpc_suppressed;
-
-	sunrpc_suppressed = write_sunrpc_kill(h, 1);
+	int ret, rc;
 
 	logger(0, 0, "Forcibly stop the Container...");
 
 	ret = ns_env_kill(h);
 	if (ret)
-		goto release_sunrpc;
+		return ret;
 
 	ret = cg_freezer_cmd(EID(h), VZCTL_CMD_FREEZE);
 	if (ret)
-		goto release_sunrpc;
+		return ret;
 
 	rc = ns_env_kill(h);
 
 	/* Unfreeze unconditionally */
 	ret = cg_freezer_cmd(EID(h), VZCTL_CMD_RESUME);
-	if (ret || rc) {
-		ret = ret ?: rc;
-		goto release_sunrpc;
-	}
+	if (ret || rc)
+		return ret ?: rc;
 
 	if (wait_env_state(h, VZCTL_ENV_STOPPED, MAX_SHTD_TM))
 		return vzctl_err(-1, 0, "Failed to stop Container:"
 				" operation timed out");
 	return 0;
-
-release_sunrpc:
-	if (sunrpc_suppressed > 0)
-		(void) write_sunrpc_kill(h, 0);
-	return ret;
 }
 
 static int ns_env_cleanup(struct vzctl_env_handle *h, int flags)

Comments

Igor Sukhih Aug. 21, 2017, 3:17 p.m.
On 08/21/2017 04:18 PM, Stanislav Kinsburskiy wrote:
> It solves two issues.
> First, it allows to kill tasks in nested namespaces (rare, but possible case).
> Second, is allows to kill CT with exited child reaper (when it's almost dead,
> but waiting for it's children), because in this case SUNRPC can't be supressed
> via child reapers proc dentries (/proc/pid/net is not available anymore).
>
> https://jira.sw.ru/browse/PSBM-70437
>
> Note: this patch reverts (replaces) commit
> f8e953f5d683aa6f04dcc2b5634444b98d6e9ee3
>
Why it can't be done in scope of env cleanup logic in the kernel?
How it going to work if Ct stop initiated not by vzctl but by OOM, halt, 
kill?

--
   Igor.
Stanislav Kinsburskiy Aug. 21, 2017, 3:24 p.m.
21.08.2017 18:17, Igor Sukhih пишет:
> On 08/21/2017 04:18 PM, Stanislav Kinsburskiy wrote:
>> It solves two issues.
>> First, it allows to kill tasks in nested namespaces (rare, but possible case).
>> Second, is allows to kill CT with exited child reaper (when it's almost dead,
>> but waiting for it's children), because in this case SUNRPC can't be supressed
>> via child reapers proc dentries (/proc/pid/net is not available anymore).
>>
>> https://jira.sw.ru/browse/PSBM-70437
>>
>> Note: this patch reverts (replaces) commit
>> f8e953f5d683aa6f04dcc2b5634444b98d6e9ee3
>>
> Why it can't be done in scope of env cleanup logic in the kernel?

Because we have suspend and can't distinguish between halt and suspend.

> How it going to work if Ct stop initiated not by vzctl but by OOM, halt, kill?

It's the same.
The intention of this SUNRPC handle is to be able to stop container with any NFS issue.
For instance with unreachable network, or when power off was called in CT (in this case network is disabled before processes exit).


> 
> -- 
>   Igor.