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

[Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>]

On 21/02/2020 16:29, Jan Beulich wrote:
> On 19.02.2020 18:25, Igor Druzhinin wrote:
>> --- a/xen/arch/x86/sysctl.c
>> +++ b/xen/arch/x86/sysctl.c
>> @@ -78,8 +78,11 @@ static void l3_cache_get(void *arg)
>>  long cpu_up_helper(void *data)
>>  {
>>      unsigned int cpu = (unsigned long)data;
>> -    int ret = cpu_up(cpu);
>> +    int ret;
>>  
>> +    /* Flush potentially scheduled RCU work from preceding CPU offline */
>> +    rcu_barrier();
>> +    ret = cpu_up(cpu);
>>      if ( ret == -EBUSY )
>>      {
>>          /* On EBUSY, flush RCU work and have one more go. */
>> @@ -104,7 +107,11 @@ long cpu_up_helper(void *data)
>>  long cpu_down_helper(void *data)
>>  {
>>      int cpu = (unsigned long)data;
>> -    int ret = cpu_down(cpu);
>> +    int ret;
>> +
>> +    /* Flush potentially scheduled RCU work from preceding CPU online */
>> +    rcu_barrier();
>> +    ret = cpu_down(cpu);
>>      if ( ret == -EBUSY )
>>      {
>>          /* On EBUSY, flush RCU work and have one more go. */
>>
> 
> There are more calls to cpu_up() / cpu_down(), most notably in
> core_parking.c. Wouldn't it be better to make the barrier part
> of the two functions? This would the also cover non-x86 in
> case an arch wants to support offlining/onlining of CPUs.

Those functions are called from early init code and rcu_barrier() is
an expensive operation. I think it's better if caller is responsible
for syncing the state. This is the reason I moved rcu_barrier() in front
of cpu_up/down.

Igor

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