linux-kernel - Re: [PATCH v3] oom-kill: add lowmem usage aware oom kill handling

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 1 Feb 2010 02:28:00 -0800 (PST)
From:	David Rientjes <rientjes@...gle.com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
cc:	Alan Cox <alan@...rguk.ukuu.org.uk>, vedran.furac@...il.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	minchan.kim@...il.com, Balbir Singh <balbir@...ux.vnet.ibm.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH v3] oom-kill: add lowmem usage aware oom kill handling

On Mon, 1 Feb 2010, KAMEZAWA Hiroyuki wrote:

> > >  109         /*
> > >  110          * Processes which fork a lot of child processes are likely
> > >  111          * a good choice. We add half the vmsize of the children if they
> > >  112          * have an own mm. This prevents forking servers to flood the
> > >  113          * machine with an endless amount of children. In case a single
> > >  114          * child is eating the vast majority of memory, adding only half
> > >  115          * to the parents will make the child our kill candidate of
> > > choice.
> > >  116          */
> > >  117         list_for_each_entry(child, &p->children, sibling) {
> > >  118                 task_lock(child);
> > >  119                 if (child->mm != mm && child->mm)
> > >  120                         points += child->mm->total_vm/2 + 1;
> > >  121                 task_unlock(child);
> > >  122         }
> > >  123
> > > ==
> > > This makes task launcher(the fist child of some daemon.) first victim.
> > 
> > That "victim", p, is passed to oom_kill_process() which does this:
> > 
> > 	/* Try to kill a child first */
> > 	list_for_each_entry(c, &p->children, sibling) {
> > 		if (c->mm == p->mm)
> > 			continue;
> > 		if (!oom_kill_task(c))
> > 			return 0;
> > 	}
> > 	return oom_kill_task(p);
> > 
> 
> Then, finally, per-process oom_adj(!=OOM_DISABLE) control is ignored ?
> Seems broken.
> 

No, oom_kill_task() returns 1 if the child has OOM_DISABLE set, meaning it 
never gets killed and we continue iterating through the child list.  If 
there are no children with seperate memory to kill, the selected task gets 
killed.  This prevents things from like sshd or bash from getting killed 
unless they are actually the memory leaker themselves.

It would naturally be better to select the child with the highest 
badness() score, but it only depends on the ordering of p->children at the 
moment.  That's because we only want to iterate through this potentially 
long list once, but improvements in this area (as well as sane tweaks to 
the heuristic) would certainly be welcome.
--
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/