[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 3/6] xen: add new cpu notifier action CPU_RESUME_FAILED



On Mon, 2019-03-25 at 13:29 +0100, Juergen Gross wrote:
> On 25/03/2019 13:21, Dario Faggioli wrote:
>
> > Reviewed-by: Dario Faggioli <dfaggioli@xxxxxxxx>
> > 
> > One more (minor) thing...
> > 
> > >  /* CPU_REMOVE: CPU was removed. */
> > > -#define CPU_REMOVE       (0x0009 | NOTIFY_REVERSE)
> > > +#define CPU_REMOVE        (0x0009 | NOTIFY_REVERSE)
> > > +/* CPU_RESUME_FAILED: CPU failed to come up in resume, all other
> > > CPUs up. */
> > > +#define CPU_RESUME_FAILED (0x000a | NOTIFY_REVERSE)
> > >  
> > ... technically, when we're dealing with CPU_RESUME_FAILED on one
> > CPU,
> > we don't know if _all_ others really went up, so I think I'd remove
> > what follows the ','.
> 
> The point is that for the CPU_RESUME_FAILED case we can be sure that
> no
> cpu will come up due to resume just a little bit later.
>
Ah, I see what you mean... that's the fact that this notifier is
invoked from another loop, and although we don't know which CPU did
manage to come up and which don't, we do know that all the ones that
could come up, are up already.

>  So we can test
> for e.g. a cpupool suddenly having no more cpus available. This is in
> contrast to CPU_UP_CANCELLED being signalled just after the one cpu
> failing to come up, but before the next cpu is triggered to come up.
> 
Right.

Well, it still looks to me that "all other CPUs up" is not entirely
accurate. But I can't propose a better alternative (unless we write
something very long, which is probably not worth it).

Perhaps you can explain this a little in another comment, like in
enable_nonboot_cpus(), before the for_each_cpu() loop itself.

But I don't feel too strong about that, and the RoB stands, whatever
you decide to do.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Software Engineer @ SUSE https://www.suse.com/


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.