Thread (18 messages) 18 messages, 4 authors, 2015-03-04

Re: [PATCH 1/3] powerpc/pseries: Simplify check for suspendability during suspend/migration

From: Nathan Fontenot <hidden>
Date: 2015-03-04 15:58:56

On 03/03/2015 02:16 PM, Tyrel Datwyler wrote:
On 03/02/2015 10:15 PM, Michael Ellerman wrote:
quoted
On Mon, 2015-03-02 at 13:30 -0800, Tyrel Datwyler wrote:
quoted
On 03/01/2015 08:19 PM, Cyril Bur wrote:
quoted
On Fri, 2015-02-27 at 18:24 -0800, Tyrel Datwyler wrote:
quoted
During suspend/migration operation we must wait for the VASI state reported
by the hypervisor to become Suspending prior to making the ibm,suspend-me
RTAS call. Calling routines to rtas_ibm_supend_me() pass a vasi_state variable
that exposes the VASI state to the caller. This is unnecessary as the caller
only really cares about the following three conditions; if there is an error
we should bailout, success indicating we have suspended and woken back up so
proceed to device tree updated, or we are not suspendable yet so try calling
rtas_ibm_suspend_me again shortly.

This patch removes the extraneous vasi_state variable and simply uses the
return code to communicate how to proceed. We either succeed, fail, or get
-EAGAIN in which case we sleep for a second before trying to call
rtas_ibm_suspend_me again.

 		u64 handle = ((u64)be32_to_cpu(args.args[0]) << 32)
 		              | be32_to_cpu(args.args[1]);
-		rc = rtas_ibm_suspend_me(handle, &vasi_rc);
-		args.rets[0] = cpu_to_be32(vasi_rc);
-		if (rc)
+		rc = rtas_ibm_suspend_me(handle);
+		if (rc == -EAGAIN)
+			args.rets[0] = cpu_to_be32(RTAS_NOT_SUSPENDABLE);
(continuing on...) so perhaps here have
	rc = 0;
else if (rc == -EIO)
	args.rets[0] = cpu_to_be32(-1);
	rc = 0;
Which should keep the original behaviour, the last thing we want to do
is break BE.
The biggest problem here is we are making what basically equates to a
fake rtas call from drmgr which we intercept in ppc_rtas(). From there
we make this special call to rtas_ibm_suspend_me() to check VASI state
and do a bunch of other specialized work that needs to be setup prior to
making the actual ibm,suspend-me rtas call. Since, we are cheating PAPR
here I guess we can really handle it however we want. I chose to simply
fail the rtas call in the case where rtas_ibm_suspend_me() fails with
something other than -EAGAIN. In user space librtas will log errno for
the failure and return RTAS_IO_ASSERT to drmgr which in turn will log
that error and fail.
We don't want to change the return values of the syscall unless we absolutely
have to. And I don't think that's the case here.
I'd like to argue that the one case I changed makes sense, but its just
as easy to keep the original behavior.
quoted
Sure we think drmgr is the only thing that uses this crap, but we don't know
for sure.
I can't imagine how anybody else could possibly use this hack without a
streamid from the hmc/hypervisor, but I've been wrong in the past more
times than I can count. :)
Correct, this will fail if called with a random streamid. The streamid has
to match what is handed to us from the HMC when a migration request is
initiated.

-Nathan
 
-Tyrel
quoted
cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help