Re: [PATCH v6 0/8] Reduce cross CPU IPI interference

[Gilad Ben-Yossef]

On Wed, Jan 11, 2012 at 9:04 AM, Milton Miller <miltonm@xxxxxxx> wrote:
>
> Hi Gilad.   A few minor corrections for several of the patch logs, but some
> meater discussions on several of the patches.
>
> Overall I like the series and hope you see it through.


Hi Milton. Thanks so much for the detailed review.

As you've no doubt noticed, English is not my mother tongue (as opposed to
 C), so a special thank you for the patch logs review :-)


>
> <SNIP>



>
> > +void on_each_cpu_mask(const struct cpumask *mask, void (*func)(void *),
> > +                     void *info, bool wait)
> > +{
> > +     int cpu = get_cpu();
> > +
> > +     smp_call_function_many(mask, func, info, wait);
> > +     if (cpumask_test_cpu(cpu, mask)) {
> > +             local_irq_disable();
> > +             func(info);
> > +             local_irq_enable();
> > +     }
> > +     put_cpu();
> > +}
> > +EXPORT_SYMBOL(on_each_cpu_mask);
>
> It should be less code if we rewrite on_each_cpu as the one liner
> on_each_cpu_mask(cpu_online_mask).  I think the trade off of less
> code is worth the cost of the added test of cpu being in online_mask.
>
> That could be a seperate patch, but will be easier to read the result
> if on_each_cpu_mask is placed above on_each_cpu in this one.


Yes, it does look cleaner and I agree that the extra test is not a big
price to pay for simplee code.

However, to do that, on_each_cpu return value need to go away and
all caller needs to be adjusted. I figured this is out of scope for this
patch set.

I did send out a separate patch set to do the needed work  (see:
https://lkml.org/lkml/2012/1/8/48) and I suggest that after both of
them go in, I'll send a patch to do exactly what you suggested.

Thanks!
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
gilad@xxxxxxxxxxxxx
Israel Cell: +972-52-8260388
US Cell: +1-973-8260388
http://benyossef.com

"Unfortunately, cache misses are an equal opportunity pain provider."
-- Mike Galbraith, LKML
--
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/