Re: [PATCH v3] mm,oom: speed up select_bad_process() loop.

[Michal Hocko <mhocko@kernel.org>]

On Thu 19-05-16 08:53:29, Michal Hocko wrote:
> On Wed 18-05-16 14:09:32, Andrew Morton wrote:
> > On Wed, 18 May 2016 16:15:45 +0200 Michal Hocko <mhocko@kernel.org> wrote:
> > 
> > > > This patch adds a counter to signal_struct for tracking how many
> > > > TIF_MEMDIE threads are in a given thread group, and check it at
> > > > oom_scan_process_thread() so that select_bad_process() can use
> > > > for_each_process() rather than for_each_process_thread().
> > > 
> > > OK, this looks correct. Strictly speaking the patch is missing any note
> > > on _why_ this is needed or an improvement. I would add something like
> > > the following:
> > > "
> > > Although the original code was correct it was quite inefficient because
> > > each thread group was scanned num_threads times which can be a lot
> > > especially with processes with many threads. Even though the OOM is
> > > extremely cold path it is always good to be as effective as possible
> > > when we are inside rcu_read_lock() - aka unpreemptible context.
> > > "
> > 
> > This sounds quite rubbery to me.  Lots of code calls
> > for_each_process_thread() and presumably that isn't causing problems. 
> 
> Yeah, many paths call for_each_process_thread but they are
> O(num_threads) while this is O(num_threads^2).

And just to clarify the regular num_threads^2 is the absolute worst case
which doesn't happen normally. We would be closer to O(num_threads) but
there is no reason to risk pathological cases when we can simply use
for_each_process to achieve the same.
-- 
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>