Re: [Xen-devel] [Patch v5 2/5] x86/hpet: Use singe apic vector rather than irq_descs for HPET interrupts

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.11.13 at 18:38, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 22/11/13 16:49, Jan Beulich wrote:
>>>>> On 22.11.13 at 17:23, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 22/11/13 15:45, Jan Beulich wrote:
>>>>>>> On 14.11.13 at 17:01, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> +/* Wake up all cpus in the channel mask.  Lock should be held. */
>>>>> +static void hpet_wake_cpus(struct hpet_event_channel *ch)
>>>>>  {
>>>>> -    unsigned int cpu = smp_processor_id();
>>>>> +    cpuidle_wakeup_mwait(ch->cpumask);
>>>>>  
>>>>> -    if ( cpumask_test_and_clear_cpu(cpu, mask) )
>>>>> -        raise_softirq(TIMER_SOFTIRQ);
>>>>> -
>>>>> -    cpuidle_wakeup_mwait(mask);
>>>>> -
>>>>> -    if ( !cpumask_empty(mask) )
>>>>> -       cpumask_raise_softirq(mask, TIMER_SOFTIRQ);
>>>>> +    if ( !cpumask_empty(ch->cpumask) )
>>>>> +       cpumask_raise_softirq(ch->cpumask, TIMER_SOFTIRQ);
>>>> Looks like the cpumask_empty() check isn't really needed here?
>>> cpuidle_wakeup_mwait(mask) will only wake cpus who have set their bit in
>>> cpuidle_mwait_flags.
>>>
>>> There might be cpus who have registered for waking up who have not yet
>>> set their bit in cpuidle_mwait_flags.
>>>
>>> Out of caution, I did by best to avoid playing with the guts of the
>>> timing code, as it seems quite fragile.
>> Some misunderstanding? The cpumask_empty() check doesn't
>> guard the call to cpuidle_wakeup_mwait(), but the one to
>> cpumask_raise_softirq() (which appears to be prepared to be
>> called with an empty mask).
> 
> cpuidle_wakeup_mwait() clears bits out of mask.  The result is the set
> of cpus who were not set in cpuidle_mwait_flags.

Sure - but there's still no big gain from doing the cpumask_empty()
check here. Unless I'm missing something...

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel