Re: [PATCH v4 2/2] mm: make lru_add_drain_all() selective

[Tejun Heo <tj@kernel.org>]

Hello, Andrew.

On Tue, Aug 13, 2013 at 03:18:05PM -0700, Andrew Morton wrote:
> I don't buy it.  The callback simply determines whether "we need to
> schuedule work on this cpu".  It's utterly simple.  Nobody will have
> trouble understanding or using such a thing.

Well, I don't buy that either.  Callback based interface has its
issues.  The difference we're talking about here is pretty minute but
then again the improvement brought on by the callback is pretty minute
too.

> It removes one memory allocation and initialisation per call.  It
> removes an entire for_each_online_cpu() loop.

But that doesn't solve the original problem at all and while it
removes the loop, it also adds a separate function.

> I really don't understand what's going on here.  You're advocating for
> a weaker kernel interface and for inferior kernel runtime behaviour. 
> Forcing callers to communicate their needs via a large,
> dynamically-allocated temporary rather than directly.  And what do we
> get in return for all this?  Some stuff about callbacks which frankly
> has me scratching my head.

Well, it is a fairly heavy path and you're pushing for an optimization
which won't make any noticeable difference at all.  And, yes, I do
think we need to stick to simpler APIs whereever possible.  Sure the
difference is minute here but the addition of test callback doesn't
buy us anything either, so what's the point?  The allocation doesn't
even exist for vast majority of configurations.  If kmalloc from that
site is problematic, the right thing to do is pre-allocating resources
on the caller side, isn't it?

Thanks.

-- 
tejun