linux-kernel - Re: [PATCH 2/4] oom: kill younger process first

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]

Message-Id: <20110512105351.a57970d7.kamezawa.hiroyu@jp.fujitsu.com>
Date:	Thu, 12 May 2011 10:53:51 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	Minchan Kim <minchan.kim@...il.com>
Cc:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	CAI Qian <caiqian@...hat.com>, avagin@...il.com,
	Andrey Vagin <avagin@...nvz.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mel Gorman <mel@....ul.ie>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, David Rientjes <rientjes@...gle.com>,
	Hugh Dickins <hughd@...gle.com>,
	Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH 2/4] oom: kill younger process first

On Thu, 12 May 2011 10:30:45 +0900
Minchan Kim <minchan.kim@...il.com> wrote:

> Hi Kame,
> 
> On Thu, May 12, 2011 at 9:52 AM, KAMEZAWA Hiroyuki
> <kamezawa.hiroyu@...fujitsu.com> wrote:
> > On Tue, 10 May 2011 17:15:01 +0900 (JST)
> > KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com> wrote:
> >
> >> This patch introduces do_each_thread_reverse() and
> >> select_bad_process() uses it. The benefits are two,
> >> 1) oom-killer can kill younger process than older if
> >> they have a same oom score. Usually younger process
> >> is less important. 2) younger task often have PF_EXITING
> >> because shell script makes a lot of short lived processes.
> >> Reverse order search can detect it faster.
> >>
> >> Reported-by: CAI Qian <caiqian@...hat.com>
> >> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
> >
> > IIUC, for_each_thread() can be called under rcu_read_lock() but
> > for_each_thread_reverse() must be under tasklist_lock.
> 
> Just out of curiosity.
> You mentioned it when I sent forkbomb killer patch. :)
> From at that time, I can't understand why we need holding
> tasklist_lock not rcu_read_lock. Sorry for the dumb question.
> 
> At present, it seems that someone uses tasklist_lock and others uses
> rcu_read_lock. But I can't find any rule for that.
> 

for_each_list_rcu() makes use of RCU list's characteristics and allows
walk a list under rcu_read_lock() without taking any atomic locks.

list_del() of RCU list works as folllowing.

==
 1) assume  A, B, C, are linked in the list.
	(head)<->(A) <-> (B)  <-> (C)

 2) remove B.
	(head)<->(A) <-> (C)
		        /
                     (B)

 Because (B)'s next points to (C) even after (B) is removed, (B)->next
 points to the alive object. Even if (C) is removed at the same time,
 (C) is not freed until rcu glace period and (C)'s next points to (head)

Then, for_each_list_rcu() can work well under rcu_read_lock(), it will visit
only alive objects (but may not be valid.)

==

please see include/linux/rculist.h and check list_add_rcu() ;)

As above implies, (B)->prev pointer is invalid pointer after list_del().
So, there will be race with list modification and for_each_list_reverse under
rcu_read__lock()

So, when you need to take atomic lock (as tasklist lock is) is...

 1) You can't check 'entry' is valid or not...
    In above for_each_list_rcu(), you may visit an object which is under removing.
    You need some flag or check to see the object is valid or not.

 2) you want to use list_for_each_safe().
    You can't do list_del() an object which is under removing...

 3) You want to walk the list in reverse.

 3) Some other reasons. For example, you'll access an object pointed by the
    'entry' and the object is not rcu safe.

make sense ?

Thanks,
-Kame


> Could you elaborate it, please?
> Doesn't it need document about it?
> 
> -- 
> Kind regards,
> Minchan Kim
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@...ck.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>
> 

--
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/