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

[Andrew Morton]

On Tue, 13 Aug 2013 18:33:04 -0400 Tejun Heo <tj@xxxxxxxxxx> wrote:

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

No it hasn't.  It's a common and simple technique which we all understand.

>  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's a relatively small improvement in the lru_add_drain_all() case. 
Other callsites can gain improvements as well.

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

It results in superior runtime code.  At this and potentially other
callsites.

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

It does buy us things, as I've repeatedly described.  You keep on
saying things which demonstrably aren't so.  I think I'll give up now.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/