Re: [Xen-devel] [PATCH v3 5/6] xen: RCU: avoid busy waiting until the end of grace period.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.08.17 at 18:06, <george.dunlap@xxxxxxxxxx> wrote:
> On 08/22/2017 02:04 PM, Jan Beulich wrote:
>>>>> On 18.08.17 at 20:04, <dario.faggioli@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/cpu/mwait-idle.c
>>> +++ b/xen/arch/x86/cpu/mwait-idle.c
>>> @@ -741,9 +741,8 @@ static void mwait_idle(void)
>>>     }
>>>  
>>>     cpufreq_dbs_timer_suspend();
>>> -
>>>     sched_tick_suspend();
>>> -   /* sched_tick_suspend() can raise TIMER_SOFTIRQ. Process it now. */
>>> +   /* Timer related operations can raise TIMER_SOFTIRQ. Process it now. */
>>>     process_pending_softirqs();
>> 
>> Is this a leftover from v1? Otherwise, why do you do the adjustment
>> here but not in acpi_processor_idle()?
>> 
>>> --- a/xen/common/rcupdate.c
>>> +++ b/xen/common/rcupdate.c
>>> @@ -84,8 +84,37 @@ struct rcu_data {
>>>      int cpu;
>>>      struct rcu_head barrier;
>>>      long            last_rs_qlen;     /* qlen during the last resched */
>>> +
>>> +    /* 3) idle CPUs handling */
>>> +    struct timer idle_timer;
>>> +    bool idle_timer_active;
>>>  };
>>>  
>>> +/*
>>> + * If a CPU with RCU callbacks queued goes idle, when the grace period is
>>> + * not finished yet, how can we make sure that the callbacks will 
> eventually
>>> + * be executed? In Linux (2.6.21, the first "tickless idle" Linux kernel),
>>> + * the periodic timer tick would not be stopped for such CPU. Here in Xen,
>>> + * we (may) don't even have a periodic timer tick, so we need to use a
>>> + * special purpose timer.
>>> + *
>>> + * Such timer:
>>> + * 1) is armed only when a CPU with an RCU callback(s) queued goes idle
>>> + *    before the end of the current grace period (_not_ for any CPUs that
>>> + *    go idle!);
>>> + * 2) when it fires, it is only re-armed if the grace period is still
>>> + *    running;
>>> + * 3) it is stopped immediately, if the CPU wakes up from idle and
>>> + *    resumes 'normal' execution.
>>> + *
>>> + * About how far in the future the timer should be programmed each time,
>>> + * it's hard to tell (guess!!). Since this mimics Linux's periodic timer
>>> + * tick, take values used there as an indication. In Linux 2.6.21, tick
>>> + * period can be 10ms, 4ms, 3.33ms or 1ms. Let's use 10ms, to enable
>>> + * at least some power saving on the CPU that is going idle.
>>> + */
>>> +#define RCU_IDLE_TIMER_PERIOD MILLISECS(10)
>> 
>> With you even mentioning that the original Linux code has ways
>> to use different values, wouldn't it be worth allowing this to be
>> command line controllable?
> 
> Apart from this, are you OK with the patch?

Yes.

> Dario is on holiday, and I think it would be good to get this
> functionality in sooner rather than later to shake out as many bugs as
> possible.  Would you be willing to let the idle timer period be set with
> a follow-up patch?

Yes.

> I'm happy to propagate the comment change to acpi_processor_idle() if
> necessary.

Propagate? So far I was of the opinion that the change had been
intentionally dropped there, but was mistakenly left in place in
mwait_idle(). Or if the comment was intended to be changed (in
both places) I can't see how that would belong into this patch, as
I think sched_tick_suspend() could have fiddled with timers even
before. But in the end I don't care all that much if the comment
adjustment gets done here or in a separate patch - my main wish
really is for the two places to stay in sync.

Jan


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