On 1/20/20 1:06 PM, David Hildenbrand wrote:
> On 17.01.20 11:46, Janosch Frank wrote:
>> The architecture specifies that processing sigp orders may be
>> asynchronous, and this is indeed the case on some hypervisors, so we
>> need to wait until the cpu runs before we return from the setup/start
>> function.
>>
>> As there was a lot of duplicate code, a common function for cpu
>> restarts has been introduced.
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
>> ---
>> lib/s390x/smp.c | 50 ++++++++++++++++++++++++++++---------------------
>> 1 file changed, 29 insertions(+), 21 deletions(-)
>>
>> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
>> index f57f420..84e681d 100644
>> --- a/lib/s390x/smp.c
>> +++ b/lib/s390x/smp.c
>> @@ -104,35 +104,46 @@ int smp_cpu_stop_store_status(uint16_t addr)
>> return rc;
>> }
>>
>> +static int smp_cpu_restart_nolock(uint16_t addr, struct psw *psw)
>> +{
>> + int rc;
>> + struct cpu *cpu = smp_cpu_from_addr(addr);
>
> I'd exchange these two (reverse christmas tree)
Christmas is over
>
>> +
>> + if (!cpu)
>> + return -1;
>
> -EINVAL?
>
>> + if (psw) {
>> + cpu->lowcore->restart_new_psw.mask = psw->mask;
>> + cpu->lowcore->restart_new_psw.addr = psw->addr;
>> + }
>
> Does this make sense to have optional? (the other CPU will execute
> random crap if not set, won't it?)
Well, I have restarts in the smp test and I don't want to always pass a
psw if I know what the last restart psw was.
Simply restarting into test_func or wait_for_flag is certainly no problem.
>
>> + rc = sigp(addr, SIGP_RESTART, 0, NULL);
>> + if (rc)
>> + return rc;
>> + /*
>> + * The order has been accepted, but the actual restart may not
>> + * have been performed yet, so wait until the cpu is running.
>> + */
>> + while (!smp_cpu_running(addr))
>> + mb();
>
> Should you make sure to stop the CPU before issuing the restart?
> Otherwise you will get false positives if it is still running (but
> hasn't processed the RESTART yet)
Good point
Attachment:
signature.asc
Description: OpenPGP digital signature
