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

Re: [Xen-devel] [PATCH 08/18] xen/arm: Disable/enable non-boot physical CPUs on suspend/resume

On 11/15/18 6:57 PM, Stefano Stabellini wrote:
On Wed, 14 Nov 2018, Julien Grall wrote:
On 14/11/2018 23:04, Stefano Stabellini wrote:
On Wed, 14 Nov 2018, Julien Grall wrote:
    -    return -ENOSYS;

Why do you return -ENOSYS until this patch? Should not have it be 0?

To distinguish that Xen suspend wasn't supported until we at least do
something useful in suspend procedure. This is not so important, we can
return 0 but needs to be fixed in previous patches.

If you return 0 before hand you can more easily bisect this series and know
where it suspend/resume breaks.

Why 0 improves bisectability? 0 prevents other checks from figuring out
that there was an error.

But this code is exactly replacing -ENOSYS by state (that would be 0 in
success. So what's wrong to return 0 rather than -ENOSYS in that patch
implement the dummy system_state?

Also, the feature is not bisectable until the
full series is applied.

Really? I was under the impression you can still do some sort of
suspend/resume patch by patch. Although, you would not do a full

You are saying that we could call the function and return successfully
even if the function does nothing, simply by returning 0. That would
make suspend bisectable within this series, patch by patch.

I think it's impressive that Mirela managed to write the series this
way, and if suspend is actually bisectable patch by patch simply by
returning 0 here, it would be amazing, and certainly worth doing.
However, if it is not the case, I wouldn't ask Mirela to make the effort
to make suspend bisectable patch by patch beyond returning 0 here, it
would be good to have but not required.

This wasn't my intention :).


Julien Grall

Xen-devel mailing list



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