On 06/01/2012 09:06 PM, Jan Beulich wrote:

quoted

quoted

quoted

On 01.06.12 at 17:13, "Srivatsa S. Bhat" [off-list ref]

wrote:

quoted

On 06/01/2012 06:29 PM, Jan Beulich wrote:

quoted

quoted

quoted

quoted

On 01.06.12 at 11:11, "Srivatsa S. Bhat" [off-list ref]

wrote:

quoted

xen_play_dead calls cpu_bringup() which looks weird, because xen_play_dead()
is invoked in the cpu down path, whereas cpu_bringup() (as the name 
suggests) is useful in the cpu bringup path.

This might not be correct - the code as it is without this change is
safe even when the vCPU gets onlined back later by an external
entity (e.g. the Xen tool stack), and it would in that case resume
at the return point of the VCPUOP_down hypercall. That might
be a heritage from the original XenoLinux tree though, and be
meaningless in pv-ops context - Jeremy, Konrad?

Possibly it was bogus/unused even in that original tree - Keir?

Thanks for your comments Jan!

In case this change is wrong, the other method I had in mind was to call
cpu_bringup_and_idle() in xen_play_dead(). (Even ARM does something similar,
in the sense that it runs the cpu bringup code including cpu_idle(), in the
cpu offline path, namely the cpu_die() function). Would that approach work
for xen as well? If yes, then we wouldn't have any issues to convert xen to
generic code.

No, that wouldn't work either afaict - the function is expected
to return.

Ok.. So, I would love to hear a confirmation about whether this patch (which
removes cpu_bringup() in xen_play_dead()) will break things or it is good as is.

If its not correct, then we can probably make __cpu_post_online() return an int,
with the meaning:

0 => success, go ahead and call cpu_idle()
non-zero => stop here, thanks for your services so far.. now leave the rest to me.

So all other archs will return 0, Xen will return non-zero, and it will handle
when to call cpu_idle() and when not to do so.

Might sound a bit ugly, but I don't see much other option. Suggestions are
appreciated!

Regards,
Srivatsa S. Bhat