Re: [Xen-devel] [PATCH v2] xen/timers: Fix memory leak with cpu unplug/plug

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 4/8/19 11:47 AM, Andrew Cooper wrote:
> On 08/04/2019 11:39, Julien Grall wrote:
> > Hi,
> >
> > On 4/8/19 10:39 AM, Andrew Cooper wrote:
> > > +    case CPU_RESUME_FAILED:
> > > +        if ( !park_offline_cpus && system_state != SYS_STATE_suspend )
> > This patch breaks compilation on arm32/arm64 because park_offline_cpus
> > is not defined:
> >
> > timer.c: In function 'cpu_callback':
> > timer.c:651:15: error: 'park_offline_cpus' undeclared (first use in
> > this function)
> >           if ( !park_offline_cpus && system_state != SYS_STATE_suspend )
> >                 ^~~~~~~~~~~~~~~~~
> >
> > What is the purpose of park_offline_cpus?
> Sorry.  I should have waited for a full build test first.
>
> park_offline_cpus is a workaround for Intel's MCE behaviour, where the
> system will shut down rather than deliver an #MC if machine checking
> isn't configured on all CPUs.
>
> As a result, we have to start all CPUs, even beyond maxcpus= and set up
> machine check handling, and never ever free their stacks, even if we'd
> prefer the CPUs to be offline.
I am a bit confused, why this is necessary now for the timer and not in 
other places of the common code?
> Are you happy with a
>
> #define park_offline_cpus false >
> in ARM?
The name is fairly confusing if you don't know the background.

But I have to admit that even with your explanation above, I still don't 
understand why you need to check park_offline_cpus in the timers.
Cheers,

--
Julien Grall

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