linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] android, lmk: Protect task->comm with task_lock.
@ 2015-08-26 12:17 Tetsuo Handa
  2015-08-26 12:19 ` [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL Tetsuo Handa
  2015-09-01 22:28 ` [PATCH 1/2] android, lmk: Protect task->comm with task_lock David Rientjes
  0 siblings, 2 replies; 10+ messages in thread
From: Tetsuo Handa @ 2015-08-26 12:17 UTC (permalink / raw)
  To: gregkh, arve, riandrews; +Cc: devel, linux-mm, mhocko, rientjes, hannes

Hello.

Next patch is mm-related but this patch is not.
Via which tree should these patches go?
----------------------------------------
>From 48c1b457eb32d7a029e9a078ee0a67974ada9261 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Wed, 26 Aug 2015 20:49:17 +0900
Subject: [PATCH 1/2] android, lmk: Protect task->comm with task_lock.

Passing task->comm to printk() wants task_lock() protection in order
to avoid potentially emitting garbage bytes.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 drivers/staging/android/lowmemorykiller.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
index 872bd60..d5d25e4 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -134,26 +134,25 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 			return 0;
 		}
 		oom_score_adj = p->signal->oom_score_adj;
-		if (oom_score_adj < min_score_adj) {
-			task_unlock(p);
-			continue;
-		}
+		if (oom_score_adj < min_score_adj)
+			goto next;
 		tasksize = get_mm_rss(p->mm);
-		task_unlock(p);
 		if (tasksize <= 0)
-			continue;
+			goto next;
 		if (selected) {
 			if (oom_score_adj < selected_oom_score_adj)
-				continue;
+				goto next;
 			if (oom_score_adj == selected_oom_score_adj &&
 			    tasksize <= selected_tasksize)
-				continue;
+				goto next;
 		}
 		selected = p;
 		selected_tasksize = tasksize;
 		selected_oom_score_adj = oom_score_adj;
 		lowmem_print(2, "select %d (%s), adj %hd, size %d, to kill\n",
 			     p->pid, p->comm, oom_score_adj, tasksize);
+next:
+		task_unlock(p);
 	}
 	if (selected) {
 		task_lock(selected);
@@ -168,10 +167,10 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 		 * task should have access to the memory reserves.
 		 */
 		mark_oom_victim(selected);
-		task_unlock(selected);
 		lowmem_print(1, "send sigkill to %d (%s), adj %hd, size %d\n",
 			     selected->pid, selected->comm,
 			     selected_oom_score_adj, selected_tasksize);
+		task_unlock(selected);
 		lowmem_deathpending_timeout = jiffies + HZ;
 		send_sig(SIGKILL, selected, 0);
 		rem += selected_tasksize;
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
  2015-08-26 12:17 [PATCH 1/2] android, lmk: Protect task->comm with task_lock Tetsuo Handa
@ 2015-08-26 12:19 ` Tetsuo Handa
  2015-08-26 15:34   ` Tetsuo Handa
  2015-09-03  1:06   ` Greg KH
  2015-09-01 22:28 ` [PATCH 1/2] android, lmk: Protect task->comm with task_lock David Rientjes
  1 sibling, 2 replies; 10+ messages in thread
From: Tetsuo Handa @ 2015-08-26 12:19 UTC (permalink / raw)
  To: gregkh, arve, riandrews; +Cc: devel, linux-mm, mhocko, rientjes, hannes

Hello.

Should selected_tasksize be added to rem even when TIF_MEMDIE was not set?

Please see a thread from http://www.spinics.net/lists/linux-mm/msg93246.html
if you want to know why to reverse the order.
----------------------------------------
>From 2d4cc11d8128e4c1397631b91fea78da3eaefb47 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Wed, 26 Aug 2015 20:52:39 +0900
Subject: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.

If we set TIF_MEMDIE before sending SIGKILL, memory reserves could be
spent for allocations which are not needed for terminating the victim.
Reverse the order as with oom_kill_process() does.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 drivers/staging/android/lowmemorykiller.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
index d5d25e4..c39b6a2 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -156,26 +156,24 @@ next:
 	}
 	if (selected) {
 		task_lock(selected);
-		if (!selected->mm) {
-			/* Already exited, cannot do mark_tsk_oom_victim() */
-			task_unlock(selected);
-			goto out;
-		}
+		lowmem_print(1, "send sigkill to %d (%s), adj %hd, size %d\n",
+			     selected->pid, selected->comm,
+			     selected_oom_score_adj, selected_tasksize);
+		task_unlock(selected);
+		send_sig(SIGKILL, selected, 0);
 		/*
 		 * FIXME: lowmemorykiller shouldn't abuse global OOM killer
 		 * infrastructure. There is no real reason why the selected
 		 * task should have access to the memory reserves.
 		 */
-		mark_oom_victim(selected);
-		lowmem_print(1, "send sigkill to %d (%s), adj %hd, size %d\n",
-			     selected->pid, selected->comm,
-			     selected_oom_score_adj, selected_tasksize);
+		task_lock(selected);
+		if (selected->mm) {
+			mark_oom_victim(selected);
+			lowmem_deathpending_timeout = jiffies + HZ;
+			rem += selected_tasksize;
+		}
 		task_unlock(selected);
-		lowmem_deathpending_timeout = jiffies + HZ;
-		send_sig(SIGKILL, selected, 0);
-		rem += selected_tasksize;
 	}
-out:
 	lowmem_print(4, "lowmem_scan %lu, %x, return %lu\n",
 		     sc->nr_to_scan, sc->gfp_mask, rem);
 	rcu_read_unlock();
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
  2015-08-26 12:19 ` [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL Tetsuo Handa
@ 2015-08-26 15:34   ` Tetsuo Handa
  2015-08-27  8:44     ` Michal Hocko
  2015-09-03  1:06   ` Greg KH
  1 sibling, 1 reply; 10+ messages in thread
From: Tetsuo Handa @ 2015-08-26 15:34 UTC (permalink / raw)
  To: gregkh, arve, riandrews; +Cc: devel, linux-mm, mhocko, rientjes, hannes

Reposting updated version as it turned out that we can call do_send_sig_info()
with task_lock held. ;-) ( http://marc.info/?l=linux-mm&m=144059948628905&w=2 )

Tetsuo Handa wrote:
> Should selected_tasksize be added to rem even when TIF_MEMDIE was not set?
Commit e1099a69a624 "android, lmk: avoid setting TIF_MEMDIE if process
has already exited" changed not to add selected_tasksize to rem. But I
noticed that rem is initialized to 0 and there is only one addition
(i.e. rem += selected_tasksize means rem = selected_tasksize) since
commit 7dc19d5affd7 "drivers: convert shrinkers to new count/scan API".
I don't know what values we should return, but this patch restores
pre commit e1099a69a624 because omitting a call to mark_oom_victim()
due to race will not prevent from reclaiming memory because we already
sent SIGKILL.
------------------------------------------------------------
>From b7075abd3a1e903e88f1755c68adc017d2125b0d Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Thu, 27 Aug 2015 00:13:57 +0900
Subject: [PATCH 2/2] android, lmk: Send SIGKILL before setting TIF_MEMDIE.

It was observed that setting TIF_MEMDIE before sending SIGKILL at
oom_kill_process() allows memory reserves to be depleted by allocations
which are not needed for terminating the OOM victim.

This patch reverts commit 6bc2b856bb7c ("staging: android: lowmemorykiller:
set TIF_MEMDIE before send kill sig"), for oom_kill_process() was updated
to send SIGKILL before setting TIF_MEMDIE.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 drivers/staging/android/lowmemorykiller.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
index d5d25e4..af604cf 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -156,26 +156,21 @@ next:
 	}
 	if (selected) {
 		task_lock(selected);
-		if (!selected->mm) {
-			/* Already exited, cannot do mark_tsk_oom_victim() */
-			task_unlock(selected);
-			goto out;
-		}
+		lowmem_print(1, "send sigkill to %d (%s), adj %hd, size %d\n",
+			     selected->pid, selected->comm,
+			     selected_oom_score_adj, selected_tasksize);
+		send_sig(SIGKILL, selected, 0);
 		/*
 		 * FIXME: lowmemorykiller shouldn't abuse global OOM killer
 		 * infrastructure. There is no real reason why the selected
 		 * task should have access to the memory reserves.
 		 */
-		mark_oom_victim(selected);
-		lowmem_print(1, "send sigkill to %d (%s), adj %hd, size %d\n",
-			     selected->pid, selected->comm,
-			     selected_oom_score_adj, selected_tasksize);
+		if (selected->mm)
+			mark_oom_victim(selected);
 		task_unlock(selected);
 		lowmem_deathpending_timeout = jiffies + HZ;
-		send_sig(SIGKILL, selected, 0);
 		rem += selected_tasksize;
 	}
-out:
 	lowmem_print(4, "lowmem_scan %lu, %x, return %lu\n",
 		     sc->nr_to_scan, sc->gfp_mask, rem);
 	rcu_read_unlock();
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
  2015-08-26 15:34   ` Tetsuo Handa
@ 2015-08-27  8:44     ` Michal Hocko
  0 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2015-08-27  8:44 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: gregkh, arve, riandrews, devel, linux-mm, rientjes, hannes

On Thu 27-08-15 00:34:47, Tetsuo Handa wrote:
> Reposting updated version as it turned out that we can call do_send_sig_info()
> with task_lock held. ;-) ( http://marc.info/?l=linux-mm&m=144059948628905&w=2 )
> 
> Tetsuo Handa wrote:
> > Should selected_tasksize be added to rem even when TIF_MEMDIE was not set?
> Commit e1099a69a624 "android, lmk: avoid setting TIF_MEMDIE if process
> has already exited" changed not to add selected_tasksize to rem. But I
> noticed that rem is initialized to 0 and there is only one addition
> (i.e. rem += selected_tasksize means rem = selected_tasksize) since
> commit 7dc19d5affd7 "drivers: convert shrinkers to new count/scan API".
> I don't know what values we should return, but this patch restores
> pre commit e1099a69a624 because omitting a call to mark_oom_victim()
> due to race will not prevent from reclaiming memory because we already
> sent SIGKILL.
> ------------------------------------------------------------
> >From b7075abd3a1e903e88f1755c68adc017d2125b0d Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Thu, 27 Aug 2015 00:13:57 +0900
> Subject: [PATCH 2/2] android, lmk: Send SIGKILL before setting TIF_MEMDIE.
> 
> It was observed that setting TIF_MEMDIE before sending SIGKILL at
> oom_kill_process() allows memory reserves to be depleted by allocations
> which are not needed for terminating the OOM victim.
> 
> This patch reverts commit 6bc2b856bb7c ("staging: android: lowmemorykiller:
> set TIF_MEMDIE before send kill sig"), for oom_kill_process() was updated
> to send SIGKILL before setting TIF_MEMDIE.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  drivers/staging/android/lowmemorykiller.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
> index d5d25e4..af604cf 100644
> --- a/drivers/staging/android/lowmemorykiller.c
> +++ b/drivers/staging/android/lowmemorykiller.c
> @@ -156,26 +156,21 @@ next:
>  	}
>  	if (selected) {
>  		task_lock(selected);
> -		if (!selected->mm) {
> -			/* Already exited, cannot do mark_tsk_oom_victim() */
> -			task_unlock(selected);
> -			goto out;
> -		}
> +		lowmem_print(1, "send sigkill to %d (%s), adj %hd, size %d\n",
> +			     selected->pid, selected->comm,
> +			     selected_oom_score_adj, selected_tasksize);
> +		send_sig(SIGKILL, selected, 0);
>  		/*
>  		 * FIXME: lowmemorykiller shouldn't abuse global OOM killer
>  		 * infrastructure. There is no real reason why the selected
>  		 * task should have access to the memory reserves.
>  		 */
> -		mark_oom_victim(selected);
> -		lowmem_print(1, "send sigkill to %d (%s), adj %hd, size %d\n",
> -			     selected->pid, selected->comm,
> -			     selected_oom_score_adj, selected_tasksize);
> +		if (selected->mm)
> +			mark_oom_victim(selected);
>  		task_unlock(selected);
>  		lowmem_deathpending_timeout = jiffies + HZ;
> -		send_sig(SIGKILL, selected, 0);
>  		rem += selected_tasksize;
>  	}
> -out:
>  	lowmem_print(4, "lowmem_scan %lu, %x, return %lu\n",
>  		     sc->nr_to_scan, sc->gfp_mask, rem);
>  	rcu_read_unlock();
> -- 
> 1.8.3.1

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] android, lmk: Protect task->comm with task_lock.
  2015-08-26 12:17 [PATCH 1/2] android, lmk: Protect task->comm with task_lock Tetsuo Handa
  2015-08-26 12:19 ` [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL Tetsuo Handa
@ 2015-09-01 22:28 ` David Rientjes
  1 sibling, 0 replies; 10+ messages in thread
From: David Rientjes @ 2015-09-01 22:28 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: gregkh, arve, riandrews, devel, linux-mm, mhocko, hannes

On Wed, 26 Aug 2015, Tetsuo Handa wrote:

> Hello.
> 
> Next patch is mm-related but this patch is not.
> Via which tree should these patches go?
> ----------------------------------------
> >From 48c1b457eb32d7a029e9a078ee0a67974ada9261 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Wed, 26 Aug 2015 20:49:17 +0900
> Subject: [PATCH 1/2] android, lmk: Protect task->comm with task_lock.
> 
> Passing task->comm to printk() wants task_lock() protection in order
> to avoid potentially emitting garbage bytes.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

We've gone through these types of patches before and Andrew has said that 
we aren't necessarily concerned with protecting task->comm here since the 
worst-case scenario is that it becomes truncated.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
  2015-08-26 12:19 ` [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL Tetsuo Handa
  2015-08-26 15:34   ` Tetsuo Handa
@ 2015-09-03  1:06   ` Greg KH
  2015-09-04 14:05     ` Michal Hocko
  1 sibling, 1 reply; 10+ messages in thread
From: Greg KH @ 2015-09-03  1:06 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: arve, riandrews, devel, linux-mm, mhocko, rientjes, hannes

On Wed, Aug 26, 2015 at 09:19:48PM +0900, Tetsuo Handa wrote:
> Hello.
> 
> Should selected_tasksize be added to rem even when TIF_MEMDIE was not set?
> 
> Please see a thread from http://www.spinics.net/lists/linux-mm/msg93246.html
> if you want to know why to reverse the order.
> ----------------------------------------
> >From 2d4cc11d8128e4c1397631b91fea78da3eaefb47 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Wed, 26 Aug 2015 20:52:39 +0900
> Subject: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
> 
> If we set TIF_MEMDIE before sending SIGKILL, memory reserves could be
> spent for allocations which are not needed for terminating the victim.
> Reverse the order as with oom_kill_process() does.


I can't take a patch that I have to hand-edit in order to apply it :(

And if we aren't taking patch 1/2, I guess this one isn't needed either?

thanks,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
  2015-09-03  1:06   ` Greg KH
@ 2015-09-04 14:05     ` Michal Hocko
  2015-09-04 17:15       ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Michal Hocko @ 2015-09-04 14:05 UTC (permalink / raw)
  To: Greg KH; +Cc: Tetsuo Handa, arve, riandrews, devel, linux-mm, rientjes, hannes

On Wed 02-09-15 18:06:20, Greg KH wrote:
[...]
> And if we aren't taking patch 1/2, I guess this one isn't needed either?

Unlike the patch1 which was pretty much cosmetic this fixes a real
issue.
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
  2015-09-04 14:05     ` Michal Hocko
@ 2015-09-04 17:15       ` Greg KH
  2015-09-04 18:06         ` Tetsuo Handa
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2015-09-04 17:15 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Tetsuo Handa, arve, riandrews, devel, linux-mm, rientjes, hannes

On Fri, Sep 04, 2015 at 04:05:59PM +0200, Michal Hocko wrote:
> On Wed 02-09-15 18:06:20, Greg KH wrote:
> [...]
> > And if we aren't taking patch 1/2, I guess this one isn't needed either?
> 
> Unlike the patch1 which was pretty much cosmetic this fixes a real
> issue.

Ok, then it would be great to get this in a format that I can apply it
in :)

thanks,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
  2015-09-04 17:15       ` Greg KH
@ 2015-09-04 18:06         ` Tetsuo Handa
  2015-09-04 20:29           ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Tetsuo Handa @ 2015-09-04 18:06 UTC (permalink / raw)
  To: gregkh, mhocko; +Cc: arve, riandrews, devel, linux-mm, rientjes, hannes

Greg KH wrote:
> On Fri, Sep 04, 2015 at 04:05:59PM +0200, Michal Hocko wrote:
> > On Wed 02-09-15 18:06:20, Greg KH wrote:
> > [...]
> > > And if we aren't taking patch 1/2, I guess this one isn't needed either?
> > 
> > Unlike the patch1 which was pretty much cosmetic this fixes a real
> > issue.
> 
> Ok, then it would be great to get this in a format that I can apply it
> in :)

I see. Here is a minimal patch.
(Acked-by: from http://lkml.kernel.org/r/20150827084443.GE14367@dhcp22.suse.cz )
----------------------------------------
>From 118609fa25700af11791b1b7e8349f8973a9e7e4 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 5 Sep 2015 02:58:12 +0900
Subject: [PATCH] android, lmk: Send SIGKILL before setting TIF_MEMDIE.

It was observed that setting TIF_MEMDIE before sending SIGKILL at
oom_kill_process() allows memory reserves to be depleted by allocations
which are not needed for terminating the OOM victim.

This patch reverts commit 6bc2b856bb7c ("staging: android: lowmemorykiller:
set TIF_MEMDIE before send kill sig"), for oom_kill_process() was updated
to send SIGKILL before setting TIF_MEMDIE.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 drivers/staging/android/lowmemorykiller.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
index 872bd60..569d12c 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -157,26 +157,22 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	}
 	if (selected) {
 		task_lock(selected);
-		if (!selected->mm) {
-			/* Already exited, cannot do mark_tsk_oom_victim() */
-			task_unlock(selected);
-			goto out;
-		}
+		send_sig(SIGKILL, selected, 0);
 		/*
 		 * FIXME: lowmemorykiller shouldn't abuse global OOM killer
 		 * infrastructure. There is no real reason why the selected
 		 * task should have access to the memory reserves.
 		 */
-		mark_oom_victim(selected);
+		if (selected->mm)
+			mark_oom_victim(selected);
 		task_unlock(selected);
 		lowmem_print(1, "send sigkill to %d (%s), adj %hd, size %d\n",
 			     selected->pid, selected->comm,
 			     selected_oom_score_adj, selected_tasksize);
 		lowmem_deathpending_timeout = jiffies + HZ;
-		send_sig(SIGKILL, selected, 0);
 		rem += selected_tasksize;
 	}
-out:
+
 	lowmem_print(4, "lowmem_scan %lu, %x, return %lu\n",
 		     sc->nr_to_scan, sc->gfp_mask, rem);
 	rcu_read_unlock();
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
  2015-09-04 18:06         ` Tetsuo Handa
@ 2015-09-04 20:29           ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2015-09-04 20:29 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: mhocko, arve, riandrews, devel, linux-mm, rientjes, hannes

On Sat, Sep 05, 2015 at 03:06:46AM +0900, Tetsuo Handa wrote:
> Greg KH wrote:
> > On Fri, Sep 04, 2015 at 04:05:59PM +0200, Michal Hocko wrote:
> > > On Wed 02-09-15 18:06:20, Greg KH wrote:
> > > [...]
> > > > And if we aren't taking patch 1/2, I guess this one isn't needed either?
> > > 
> > > Unlike the patch1 which was pretty much cosmetic this fixes a real
> > > issue.
> > 
> > Ok, then it would be great to get this in a format that I can apply it
> > in :)
> 
> I see. Here is a minimal patch.
> (Acked-by: from http://lkml.kernel.org/r/20150827084443.GE14367@dhcp22.suse.cz )
> ----------------------------------------
> >From 118609fa25700af11791b1b7e8349f8973a9e7e4 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Sat, 5 Sep 2015 02:58:12 +0900
> Subject: [PATCH] android, lmk: Send SIGKILL before setting TIF_MEMDIE.
> 
> It was observed that setting TIF_MEMDIE before sending SIGKILL at
> oom_kill_process() allows memory reserves to be depleted by allocations
> which are not needed for terminating the OOM victim.
> 
> This patch reverts commit 6bc2b856bb7c ("staging: android: lowmemorykiller:
> set TIF_MEMDIE before send kill sig"), for oom_kill_process() was updated
> to send SIGKILL before setting TIF_MEMDIE.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Acked-by: Michal Hocko <mhocko@suse.com>
> ---
>  drivers/staging/android/lowmemorykiller.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)

Please send this in a format that I can apply it in that doesn't require
me to hand-edit the email :(

thanks,

greg k-h

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-09-04 20:29 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-26 12:17 [PATCH 1/2] android, lmk: Protect task->comm with task_lock Tetsuo Handa
2015-08-26 12:19 ` [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL Tetsuo Handa
2015-08-26 15:34   ` Tetsuo Handa
2015-08-27  8:44     ` Michal Hocko
2015-09-03  1:06   ` Greg KH
2015-09-04 14:05     ` Michal Hocko
2015-09-04 17:15       ` Greg KH
2015-09-04 18:06         ` Tetsuo Handa
2015-09-04 20:29           ` Greg KH
2015-09-01 22:28 ` [PATCH 1/2] android, lmk: Protect task->comm with task_lock David Rientjes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox