diff mbox

[1/1,Y/Z,SRU] pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns_processes

Message ID 20170619130217.27308-2-seth.forshee@canonical.com
State New
Headers show

Commit Message

Seth Forshee June 19, 2017, 1:02 p.m. UTC
From: "Eric W. Biederman" <ebiederm@xmission.com>

BugLink: http://bugs.launchpad.net/bugs/1698264

The code can potentially sleep for an indefinite amount of time in
zap_pid_ns_processes triggering the hung task timeout, and increasing
the system average.  This is undesirable.  Sleep with a task state of
TASK_INTERRUPTIBLE instead of TASK_UNINTERRUPTIBLE to remove these
undesirable side effects.

Apparently under heavy load this has been allowing Chrome to trigger
the hung time task timeout error and cause ChromeOS to reboot.

Reported-by: Vovo Yang <vovoy@google.com>
Reported-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Fixes: 6347e9009104 ("pidns: guarantee that the pidns init will be the last pidns process reaped")
Cc: stable@vger.kernel.org
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
(cherry picked from commit b9a985db98961ae1ba0be169f19df1c567e4ffe0)
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 kernel/pid_namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Colin Ian King June 19, 2017, 1:07 p.m. UTC | #1
On 19/06/17 14:02, Seth Forshee wrote:
> From: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1698264
> 
> The code can potentially sleep for an indefinite amount of time in
> zap_pid_ns_processes triggering the hung task timeout, and increasing
> the system average.  This is undesirable.  Sleep with a task state of
> TASK_INTERRUPTIBLE instead of TASK_UNINTERRUPTIBLE to remove these
> undesirable side effects.
> 
> Apparently under heavy load this has been allowing Chrome to trigger
> the hung time task timeout error and cause ChromeOS to reboot.
> 
> Reported-by: Vovo Yang <vovoy@google.com>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> Fixes: 6347e9009104 ("pidns: guarantee that the pidns init will be the last pidns process reaped")
> Cc: stable@vger.kernel.org
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> (cherry picked from commit b9a985db98961ae1ba0be169f19df1c567e4ffe0)
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  kernel/pid_namespace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index a65ba137fd15..567ecc826bc8 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -255,7 +255,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
>  	 * if reparented.
>  	 */
>  	for (;;) {
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> +		set_current_state(TASK_INTERRUPTIBLE);
>  		if (pid_ns->nr_hashed == init_pids)
>  			break;
>  		schedule();
> 

Clean upstream cherry pick, test results look good.

Acked-by: Colin Ian King <colin.king@canonical.com>
Stefan Bader June 21, 2017, 8:08 a.m. UTC | #2
On 19.06.2017 15:02, Seth Forshee wrote:
> From: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1698264
> 
> The code can potentially sleep for an indefinite amount of time in
> zap_pid_ns_processes triggering the hung task timeout, and increasing
> the system average.  This is undesirable.  Sleep with a task state of
> TASK_INTERRUPTIBLE instead of TASK_UNINTERRUPTIBLE to remove these
> undesirable side effects.
> 
> Apparently under heavy load this has been allowing Chrome to trigger
> the hung time task timeout error and cause ChromeOS to reboot.
> 
> Reported-by: Vovo Yang <vovoy@google.com>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> Fixes: 6347e9009104 ("pidns: guarantee that the pidns init will be the last pidns process reaped")
> Cc: stable@vger.kernel.org
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> (cherry picked from commit b9a985db98961ae1ba0be169f19df1c567e4ffe0)
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---
>  kernel/pid_namespace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index a65ba137fd15..567ecc826bc8 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -255,7 +255,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
>  	 * if reparented.
>  	 */
>  	for (;;) {
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> +		set_current_state(TASK_INTERRUPTIBLE);
>  		if (pid_ns->nr_hashed == init_pids)
>  			break;
>  		schedule();
>
Stefan Bader June 21, 2017, 11:28 a.m. UTC | #3
Applied to Yakkety and Zesty master-next.

Thanks,
-Stefan
diff mbox

Patch

diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index a65ba137fd15..567ecc826bc8 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -255,7 +255,7 @@  void zap_pid_ns_processes(struct pid_namespace *pid_ns)
 	 * if reparented.
 	 */
 	for (;;) {
-		set_current_state(TASK_UNINTERRUPTIBLE);
+		set_current_state(TASK_INTERRUPTIBLE);
 		if (pid_ns->nr_hashed == init_pids)
 			break;
 		schedule();