On Thu, 16 Jan 2020 15:44:24 +0100
Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
> On 1/16/20 3:14 PM, Cornelia Huck wrote:
> > On Thu, 16 Jan 2020 07:05:10 -0500
> > Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:
> >> +static int smp_cpu_restart_nolock(uint16_t addr, struct psw *psw)
> >> +{
> >> + int rc;
> >> + struct cpu *cpu = smp_cpu_from_addr(addr);
> >> +
> >> + if (!cpu)
> >> + return -1;
> >> + if (psw) {
> >> + cpu->lowcore->restart_new_psw.mask = psw->mask;
> >> + cpu->lowcore->restart_new_psw.addr = psw->addr;
> >> + }
> >> + rc = sigp(addr, SIGP_RESTART, 0, NULL);
> >> + if (rc)
> >> + return rc;
> >> + while (!smp_cpu_running(addr)) { mb(); }
> >
> > Maybe split this statement? Also, maybe add a comment
>
> /* Wait until the target cpu is running */
> ?
Fine with me as well :)
>
> This is not QEMU with two line ifs taking up 3 lines :)
Heh, it's just the style I'm used to :)
>
> >
> > /*
> > * The order has been accepted, but the actual restart may not
> > * have been performed yet, so wait until the cpu is running.
> > */
> >
> > ?
> >
> >> + cpu->active = true;
> >> + return 0;
> >> +}
> >
> > The changes look good to me AFAICS.
> >
> > Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
>
> Thanks!
>
>
Attachment:
pgpzKMzxqF1zU.pgp
Description: OpenPGP digital signature
