On Sat, 16 Apr 2011, Tejun Heo wrote:
> > + new = 0;
> > + }
> > +#ifdef CONFIG_PREEMPT
> > + } while (this_cpu_cmpxchg(*fbc->counters, count, new) != count);
> > +#else
> > + } while (0);
> > + this_cpu_write(*fbc->counters, new);
> > +#endif
>
> Eeeek, no. If you want to do the above, please put it in a separate
> inline function with sufficient comment.
That would not work well with the control flow. Just leave the cmpxchg for
both cases? That would make the function irq safe as well!
> > + if (unlikely(overflow)) {
> > spin_lock(&fbc->lock);
> > - fbc->count += count;
> > - __this_cpu_write(*fbc->counters, 0);
> > + fbc->count += overflow;
> > spin_unlock(&fbc->lock);
>
> Why put this outside and use yet another branch?
Because that way we do not need preempt enable/disable. The cmpxchg is
used to update the per cpu counter in the slow case as well. All that is
left then is to add the count to the global counter.
The branches are not an issue since they are forward branches over one
(after converting to an atomic operation) or two instructions each. A
possible stall is only possible in case of the cmpxchg failing.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx 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@xxxxxxxxx";> email@xxxxxxxxx </a>