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
|
Home