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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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?


> > 
> > Signed-off-by: Mirela Simonovic <mirela.simonovic@xxxxxxxxxx>
> > ---
> >   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
> 

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