Re: [RFC PATCH v2 3/3] sched: introduce synchronized idle injection

Jacob Pan
2015-11-10T18:42:59Z

On Tue, 10 Nov 2015 14:23:24 +0100
Peter Zijlstra <peterz@infradead.org> wrote:

> > +/* protect injection parameters from runtime changes */
> > +static DEFINE_SPINLOCK(idle_inject_lock);
>
> A global lock, yay :-), I think you want this to be a RAW_SPINLOCK
> though. As on -RT this would want to actually run from IRQ context
> too.
>
I am not using this lock in per cpu timer handler, I guess it would not
be in IRQ context?
> > + if (bitmap_weight(idle_inject_cpumask,
> > num_possible_cpus())) {
>
> I don't get the point of this bitmap; with the cpu notifier you
> basically ensure this is equal to online_mask.
>
> Also, this weight test is pointless, if the bitmap is empty the
> for_each_set_bit() should be of equal cost -- and afaict nothing
> calling this is performance critical in the first place.
Agreed, I will remove that. I was planning on a subset of CPUs but not
for now, e.g. a socket or domain.