Re: [Xen-devel] [PATCH 4/6] xen: don't free percpu areas during suspend

[Juergen Gross <jgross@xxxxxxxx>]

On 28/03/2019 09:03, Jan Beulich wrote:
>>>> On 28.03.19 at 07:59, <jgross@xxxxxxxx> wrote:
>> On 27/03/2019 17:52, Juergen Gross wrote:
>>> On 27/03/2019 17:38, Jan Beulich wrote:
>>>>>>> On 27.03.19 at 17:18, <jgross@xxxxxxxx> wrote:
>>>>> On 27/03/2019 16:55, Andrew Cooper wrote:
>>>>>> On 18/03/2019 13:11, Juergen Gross wrote:
>>>>>>> Instead of freeing percpu areas during suspend and allocating them
>>>>>>> again when resuming keep them. Only free an area in case a cpu didn't
>>>>>>> come up again when resuming.
>>>>>>>
>>>>>>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
>>>>>>
>>>>>> Hmm - this is slightly problematic, given the dual nature of this code.
>>>>>>
>>>>>> I agree that it this change is beneficial for the suspend case, but it
>>>>>> is a problem when we are parking an individual CPU for smt=0 or
>>>>>> xen-hptool reasons.
>>>>>>
>>>>>> Do we have any hint we can use when taking the CPU down as to whether
>>>>>> we're expecting it to come straight back up again?
>>>>>
>>>>> Did you look into the patch? I did this by testing system_state.
>>>>
>>>> I think there's a wider problem here: enable_nonboot_cpus()
>>>> only brings back up the CPUs that were previously online.
>>>> Parked ones would be left alone, yet after resume they'd
>>>> need to be put back into parked state.
>>>
>>> I can add that handling in the respin of the series.
>>
>> Looking deeper into that mess I believe that should be a series of its
>> own. Cpu parking needs to be handled for cpu hotplug and core parking
>> (XENPF_core_parking), too.
> 
> What issue do you see for CPU hotplug? cpu_up_helper() has
> been modified by the parking series.

I was thinking of hot unplug. cpu_down() won't do the job for a parked
cpu.

> For core parking I wonder whether core_parking_helper()
> shouldn't, first of all, invoke cpu_{up,down}_helper(). This
> wouldn't be enough, though - the policy hooks need to honor
> opt_smt as well.

Right.

> As to this wanting to be a patch / series of its own - I don't
> mind, but preferably it would come ahead of your changes
> here, so that it can be backported independently and
> (sufficiently) easily (unless of course there's really no
> collision between the two).

In case there is a collision it should be fairly minimal.

I'd prefer not to block my series as it is a prerequisite for my core
scheduling series, which I believe should go in rather sooner than later
as it probably should see lot of testing before the next release.


Juergen

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