Re: [Xen-devel] [PATCH v3] x86/cpu: Sync any remaining RCU callbacks before CPU up/down

[Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>]

On 06/03/2020 09:43, Jan Beulich wrote:
> On 04.03.2020 16:33, Igor Druzhinin wrote:
>> --- a/xen/arch/x86/acpi/power.c
>> +++ b/xen/arch/x86/acpi/power.c
>> @@ -305,7 +305,6 @@ static int enter_state(u32 state)
>>      cpufreq_add_cpu(0);
>>  
>>   enable_cpu:
>> -    rcu_barrier();
>>      mtrr_aps_sync_begin();
>>      enable_nonboot_cpus();
>>      mtrr_aps_sync_end();
> 
> I take it you remove the invocation here because of being redundant
> with the cpu_up() in enable_nonboot_cpus(). Is this safe / correct
> in all cases? For one, it's not obvious to me that
> mtrr_aps_sync_begin() couldn't rely on RCU syncing to have happened.

From the history (9d9af7dca878), rcu_barrier there was introduce for
exactly same reason I put it into cpu_up/down. I'm pretty certain
it's safe as there is no other obvious reason to have it here.

The only function that could affect mtrr_aps_sync_begin() is
mtrr_aps_sync_end() and that one is called only below.

> And then enable_nonboot_cpus() may not call cpu_up() at all,
> because of the park_offline_cpus-based early loop continuation in
> the function.

I can't see how that is related.

>> --- a/xen/arch/x86/sysctl.c
>> +++ b/xen/arch/x86/sysctl.c
>> @@ -85,11 +85,7 @@ long cpu_up_helper(void *data)
>>      int ret = cpu_up(cpu);
>>  
>>      if ( ret == -EBUSY )
>> -    {
>> -        /* On EBUSY, flush RCU work and have one more go. */
>> -        rcu_barrier();
>>          ret = cpu_up(cpu);
>> -    }
>>  
>>      if ( !ret && !opt_smt &&
>>           cpu_data[cpu].compute_unit_id == INVALID_CUID &&
>> @@ -110,11 +106,7 @@ long cpu_down_helper(void *data)
>>      int cpu = (unsigned long)data;
>>      int ret = cpu_down(cpu);
>>      if ( ret == -EBUSY )
>> -    {
>> -        /* On EBUSY, flush RCU work and have one more go. */
>> -        rcu_barrier();
>>          ret = cpu_down(cpu);
>> -    }
> 
> In both cases I think the comments would better be retained (in
> an adjusted shape).

Ok.

Igor

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel