lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sun, 28 Mar 2010 22:55:28 +0800 From: anfei <anfei.zhou@...il.com> To: Oleg Nesterov <oleg@...hat.com> Cc: Andrew Morton <akpm@...ux-foundation.org>, rientjes@...gle.com, kosaki.motohiro@...fujitsu.com, nishimura@....nes.nec.co.jp, kamezawa.hiroyu@...fujitsu.com, linux-mm@...ck.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH] oom killer: break from infinite loop On Fri, Mar 26, 2010 at 11:33:56PM +0100, Oleg Nesterov wrote: > On 03/26, Andrew Morton wrote: > > > > On Thu, 25 Mar 2010 00:25:05 +0800 > > Anfei Zhou <anfei.zhou@...il.com> wrote: > > > > > --- a/mm/oom_kill.c > > > +++ b/mm/oom_kill.c > > > @@ -381,6 +381,8 @@ static void dump_header(struct task_struct *p, gfp_t gfp_mask, int order, > > > */ > > > static void __oom_kill_task(struct task_struct *p, int verbose) > > > { > > > + struct task_struct *t; > > > + > > > if (is_global_init(p)) { > > > WARN_ON(1); > > > printk(KERN_WARNING "tried to kill init!\n"); > > > @@ -412,6 +414,8 @@ static void __oom_kill_task(struct task_struct *p, int verbose) > > > */ > > > p->rt.time_slice = HZ; > > > set_tsk_thread_flag(p, TIF_MEMDIE); > > > + for (t = next_thread(p); t != p; t = next_thread(t)) > > > + set_tsk_thread_flag(t, TIF_MEMDIE); > > > > > > force_sig(SIGKILL, p); > > > > Don't we need some sort of locking while walking that ring? > > This should be always called under tasklist_lock, I think. > At least this seems to be true in Linus's tree. > Yes, this function is always called with read_lock(&tasklist_lock), so it should be okay. > I'd suggest to do > > - set_tsk_thread_flag(p, TIF_MEMDIE); > + t = p; > + do { > + set_tsk_thread_flag(t, TIF_MEMDIE); > + } while_each_thread(p, t); > > but this is matter of taste. > Yes, this is better. > Off-topic, but we shouldn't use force_sig(), SIGKILL doesn't > need "force" semantics. > This may need a dedicated patch, there are some other places to force_sig(SIGKILL, ...) too. > I'd wish I could understand the changelog ;) > Assume thread A and B are in the same group. If A runs into the oom, and selects B as the victim, B won't exit because at least in exit_mm(), it can not get the mm->mmap_sem semaphore which A has already got. So no memory is freed, and no other task will be selected to kill. I formatted the patch for -mm tree as David suggested. --- mm/oom_kill.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -418,8 +418,15 @@ static void dump_header(struct task_stru */ static void __oom_kill_task(struct task_struct *p) { + struct task_struct *t; + p->rt.time_slice = HZ; - set_tsk_thread_flag(p, TIF_MEMDIE); + + t = p; + do { + set_tsk_thread_flag(t, TIF_MEMDIE); + } while_each_thread(p, t); + force_sig(SIGKILL, p); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists