Re: [Xen-devel] [PATCH 1/2] enable event channel wake-up for mem_event interfaces

[Tim Deegan <tim@xxxxxxx>]

At 13:18 -0400 on 13 Oct (1318511910), Adin Scannell wrote:
> I'll rework the patches incorporating the feedback and resend the
> modified patches.
> 
> I've got a couple of quick notes, inline below.
> 
> >> @@ -135,16 +166,28 @@ void mem_event_put_request(struct domain
> >> +    /*
> >> +     * We ensure that each vcpu can put at least *one* event -- because 
> >> some
> >> +     * events are not repeatable, such as dropping a page.  This will 
> >> ensure no
> >> +     * vCPU is left with an event that they must place on the ring, but 
> >> cannot.
> >> +     * They will be paused after the event is placed.
> >> +     * See large comment below in mem_event_unpause_vcpus().
> >> +     */
> >> +    if( current->domain->domain_id == d->domain_id &&
> >> +        mem_event_ring_free(d, med) < d->max_vcpus )
> >> +        mem_event_mark_and_pause(current, med);
> >> +
> >
> > This idiom of comparing domain-ids cropped up in the earlier mem-event
> > patches and seems to be spreading, but the right check is just
> > (current->domain == d).
> >
> > Also: are there cases where current->domain != d?  If so, can't those cases
> > cause the ring to fill up?
> 
> Yes, I believe there are there are cases where a different domain
> (i.e. the domain w/ the device model) can map a page generating an
> event (mapping a paged-out page, writeable mappings of pages with
> non-writable access bits, etc.).  Unfortunately, we can't pause any
> vcpu in those cases.

True.

> This is something that I intend to revisit, as guaranteeing that
> absolutely no events are lost may be quite complicated (especially
> when there are certain events which are not repeatable). I'm
> considering the use of the new wait queues or other mechanisms to wait
> for the ring to clear up while in the same context.... but that is
> another sort of tricky.

Yep.  You could do what the timer code does in this situation and have a
linked list of events that didnt make it on to the queue (to be tidied
up later when there's space) but I can see that being messy too, and 
in extremis you might not be able to xmalloc another entry...

> > Is there a risk that under heavy mem-event loads vcpu 0 might starve
> > other vcpus entirely because they're never allowed to unpause here?
> 
> Unfortunately, yes.  With heavy mem event load (& a dom0 that isn't
> taking them off the ring fast enough).  I think there are two fair
> alternatives:
> 1) Unpausing a vcpu at random.
> 2) Waiting until the ring reaches a watermark and unpausing all vCPUs.
> 
> Any thoughts on these?

3) always start the scan from a different place (either a round-robin or
from the last-serviced-vcpu + 1)?

Cheers,

Tim.

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