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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Andrew,

On 4/8/19 1:09 PM, Andrew Cooper wrote:
> On 08/04/2019 12:38, Julien Grall wrote:
> > 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.
> It is all to do with how/when we free per-cpu data.
>
> Technically speaking (with the memory leak fixed) the old arrangement
> ought to function correctly, but the new arrangement is more efficient.
Where would the free happen in the "less efficient" way?

Cheers,

--
Julien Grall

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