Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
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.ge14...@dhcp22.suse.cz ) >From 118609fa25700af11791b1b7e8349f8973a9e7e4 Mon Sep 17 00:00:00 2001 From: Tetsuo HandaDate: 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 Acked-by: Michal Hocko --- 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
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.ge14...@dhcp22.suse.cz ) > > >From 118609fa25700af11791b1b7e8349f8973a9e7e4 Mon Sep 17 00:00:00 2001 > From: Tetsuo Handa> 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 > Acked-by: Michal Hocko > --- > 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
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> 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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
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-mmm=144059948628905w=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-ker...@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-ker...@i-love.sakura.ne.jp Acked-by: Michal Hocko mho...@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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] android, lmk: Reverse the order of setting TIF_MEMDIE and sending SIGKILL.
Reposting updated version as it turned out that we can call do_send_sig_info() with task_lock held. ;-) ( http://marc.info/?l=linux-mmm=144059948628905w=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-ker...@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-ker...@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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel