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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

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?

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?

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

 -George

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