Re: [PATCH 4/7] xen/arm: When CPU dies, free percpu area immediatelly

[Julien Grall <julien.grall@arm.com>]

On 12/04/18 00:46, Stefano Stabellini wrote:
> On Wed, 11 Apr 2018, Julien Grall wrote:
>> On 11/04/18 14:19, Mirela Simonovic wrote:
>>> Freeing percpu area is done when a non-boot CPU is disabled upon suspend.
>>> This use to be scheduled for execution after a period of time, what caused
>>> the following racing issues. If CPU is enabled after it is disabled and
>>> before the freeing of percpu area is performed, Xen would crash upon
>>> initializing percpu area because per cpu offset is not marked as
>>> INVALID_PERCPU_AREA (this suppose to happen when cpu area is freed).
>>> To resolve the racing issue, free percpu area right away instead
>>> scheduling it for later.
>>
>> The reason of using the RCU is you want to make sure that none of the other
>> CPUs will access that percpu data before freeing it. So I don't think this
>> patch is valid.
>>
>> It looks like to me a rcu barrier is missing after calling cpu_down somewhere
>> in the CPU off path. I am not entirely sure where.
> 
> We need a rcu_barrier(). Perhaps, it could be added on cpu_on before
> initializing the percpu area?

Do you mind giving a bit more details on your thought? cpu_up looks a 
strange place as no one should access the percpu area after the CPU is 
down. So it feels the rcu_barrier should be somewhere before 
PSCI_cpu_off is called.

Cheers,

> 
> 
>>>
>>> Signed-off-by: Mirela Simonovic <mirela.simonovic@aggios.com>
>>> ---
>>>    xen/arch/arm/percpu.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/arm/percpu.c b/xen/arch/arm/percpu.c
>>> index 25442c48fe..e4e8405f43 100644
>>> --- a/xen/arch/arm/percpu.c
>>> +++ b/xen/arch/arm/percpu.c
>>> @@ -46,7 +46,7 @@ static void free_percpu_area(unsigned int cpu)
>>>    {
>>>        struct free_info *info = &per_cpu(free_info, cpu);
>>>        info->cpu = cpu;
>>> -    call_rcu(&info->rcu, _free_percpu_area);
>>> +    _free_percpu_area(&info->rcu);
>>>    }
>>>      static int cpu_percpu_callback(
>>>
>>
>> -- 
>> Julien Grall
>>

-- 
Julien Grall

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