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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.03.2020 14:10, Igor Druzhinin wrote:
> 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.

Oh, I didn't go do archeology here. This is then certainly fine.

> 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.

It could be without your observation above, as there could have
been other reasons to have the barrier there (and hence it then
could be a bug if the barrier was removed from an effective
code path, rather than just moved).

Jan

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