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
-Tyrelquoted
cheers