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

[Julien Grall <julien.grall@xxxxxxx>]

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@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
--
Julien Grall

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