Re: [PATCH v07 2/9] hotplug/cpu: Add operation queuing function

[Michael Ellerman <mpe@ellerman.id.au>]

Michael Bringmann <mwb@linux.vnet.ibm.com> writes:
> On 07/23/2018 12:51 PM, Nathan Fontenot wrote:
>> On 07/13/2018 03:18 PM, Michael Bringmann wrote:
...
>>> +
>>> +int dlpar_queued_actions_run(void)
>>> +{
>>> +=C2=A0=C2=A0=C2=A0 if (!list_empty(&dlpar_delayed_list)) {
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct pseries_hp_errorlog =
hp_errlog;
>>> +
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 hp_errlog.resource =3D PSER=
IES_HP_ELOG_RESOURCE_PMT;
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 hp_errlog.action =3D 0;
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 hp_errlog.id_type =3D 0;
>>> +
>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 queue_hotplug_event(&hp_err=
log, 0, 0); > +=C2=A0=C2=A0=C2=A0 }
>>> +=C2=A0=C2=A0=C2=A0 return 0;
>>> +}
>>=20
>> I'm a bit confused by this. Is there a reason this needs to queue a
>> hotplug event instead of just walking the list as is done in dlpar_pmt?
>
> Up to this point, the operations have only been added to 'dlpar_delayed_l=
ist'.
> This function separates the execution of the CPU readd and Memory readd
> operations from the execution of 'migration store'.  If we walk the list
> here, then we add the execution time of all of the readd operations to
> the time of 'migration store'.  This is not a large problem in small
> systems like we have in the Kernel Team.  This may be a major issue thoug=
h,
> for production SAP HANA systems where we may be readding thousands of pag=
es
> of memory.  By pushing the execution of the CPU readd and Memory readd
> operations after, and separate, from the execution of 'migration store',
> we do not delay the end of the operation or the return of the completion
> status to an associated HMC.

But you can't return completion status, because the operation hasn't
completed.

The migration isn't finished until we've updated the topology for the
new system.

If you decouple things like this you now have no way of reporting
progress or an error to the caller.

So I'm unconvinced this is the right solution.

cheers