Re: [Xen-devel] [PATCH v2 3/6] xen: RCU/x86/ARM: discount CPUs that were idle when grace period started.

[Tim Deegan <tim@xxxxxxx>]

Hi,

This looks good to me.  I have one question:

At 18:45 +0200 on 16 Aug (1502909149), Dario Faggioli wrote:
> @@ -474,7 +484,41 @@ static struct notifier_block cpu_nfb = {
>  void __init rcu_init(void)
>  {
>      void *cpu = (void *)(long)smp_processor_id();
> +
> +    cpumask_setall(&rcu_ctrlblk.idle_cpumask);
> +    /* The CPU we're running on is certainly not idle */
> +    cpumask_clear_cpu(smp_processor_id(), &rcu_ctrlblk.idle_cpumask);
>      cpu_callback(&cpu_nfb, CPU_UP_PREPARE, cpu);
>      register_cpu_notifier(&cpu_nfb);
>      open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
>  }
> +
> +/*
> + * The CPU is becoming idle, so no more read side critical
> + * sections, and one more step toward grace period.
> + */
> +void rcu_idle_enter(unsigned int cpu)
> +{
> +    /*
> +     * During non-boot CPU bringup and resume, until this function is
> +     * called for the first time, it's fine to find our bit already set.
> +     */
> +    ASSERT(!cpumask_test_cpu(cpu, &rcu_ctrlblk.idle_cpumask) ||
> +           (system_state < SYS_STATE_active || system_state >= 
> SYS_STATE_resume));

Does every newly started CPU immediately idle?  If not, then it might
run in an RCU read section but excluded from the grace period
mechanism.

It seems like it would be better to start with the idle_cpumask empty,
and rely on online_cpumask to exclude CPUs that aren't running.
Or if that doesn't work, to call rcu_idle_exit/enter on the CPU
bringup/shutdown paths and simplify this assertion.

Cheers,

Tim.

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