Re: [Xen-devel] [PATCH] xc_save: ignore the first suspend event channel notification

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

On 5/9/08 20:16, "Brendan Cully" <brendan@xxxxxxxxx> wrote:

> I've noticed that the suspend event channel becomes pending as soon as
> it is bound. I'm not sure why or whether this is intentional, but it
> means that the suspend function will return before the domain has
> completed suspending unless the first notification is cleared. Without
> this patch, xc_domain_save may find that the guest has not suspended
> and sleep in 10ms chunks until it does. Typically this is several
> milliseconds of wasted time.

This patch looks pretty dodgy to me.

First of all, you removed the non-error-case return statement from
suspend_evtchn_init(). Hence it appears you release the local port before
returning, and you return -1 (which you'd never notice since the caller
erroneously does not check the return code - please fix this).

Hence I think actually you are ending up using the old slow shutdown method
since suspend_evtchn will be -1 and hence you'll end up in compat_suspend().
That should result in a significant slowdown rather than speedup, so were
your measurements actually taken with this exact patch?

Even with this fixed, I don't think that ignoring an event wakeup is a great
idea. You can easily make the not-suspended-yet retry loop always wait on
the evtchn rather than sleeping 10ms. By which I mean -- there is really no
reason for that to be a pure poll loop when you have an evtchn to sleep on.

To do this, call (*suspend) from within the retry loop: the evtchn case can
do what it always does (basically sleep on the evtchn device until its
evtchn of interest appears); the compat case should change behaviour after
its first invocation so that it sleeps 10ms (stash a static variable in the
function or in suspendinfo for this purpose, to remember whether it was
already invoked).

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel