Re: [PATCH RFC 0/6] Slotted channels for sync vm_events

[Tamas K Lengyel <tamas@tklengyel.com>]

On Thu, Dec 20, 2018 at 3:48 AM Petre Ovidiu PIRCALABU
<ppircalabu@bitdefender.com> wrote:
>
> On Wed, 2018-12-19 at 15:33 -0700, Tamas K Lengyel wrote:
> > On Wed, Dec 19, 2018 at 11:52 AM Petre Pircalabu
> > <ppircalabu@bitdefender.com> wrote:
> > >
> > > This patchset is a rework of the "multi-page ring buffer" for
> > > vm_events
> > > patch based on Andrew Cooper's comments.
> > > For synchronous vm_events the ring waitqueue logic was unnecessary
> > > as the
> > > vcpu sending the request was blocked until a response was received.
> > > To simplify the request/response mechanism, an array of slotted
> > > channels
> > > was created, one per vcpu. Each vcpu puts the request in the
> > > corresponding slot and blocks until the response is received.
> > >
> > > I'm sending this patch as a RFC because, while I'm still working on
> > > way to
> > > measure the overall performance improvement, your feedback would be
> > > a great
> > > assistance.
> >
> > Generally speaking this approach is OK, but I'm concerned that we
> > will
> > eventually run into the same problem that brought up the idea of
> > using
> > multi-page rings: vm_event structures that are larger then a page.
> > Right now this series adds a ring for each vCPU, which does mitigate
> > some of the bottleneck, but it does not really address the root
> > cause.
> > It also adds significant complexity as the userspace side now has to
> > map in multiple rings, each with its own event channel and polling
> > requirements.
> >
> > Tamas
> The memory for the vm_event "rings" (actually for synchronous vm_event
> just an array of vm_event_slot structures ( state + vm_event_request /
> vm_event_response) is allocated directly from domheap and spans over as
> many pages as necessary.

Ah, OK, I missed that. In that case that is fine :)

> Regarding the userspace complexity, unfortunately I haven't had a
> better idea (but I'm open to suggestions).
> In order to have a lock-free mechanism to access the vm_event data,
> each vcpu should access only its own slot (referenced by vcpu_id).
> I have used the "one event channel per slot + one for the async ring"
> approach, because, to my understanding, the only additional information
> an event channel can carry is the vcpu on which is triggered.

Right, alternative would be to have a single event channel and then
the userspace has to check each slot manually to see which was
updated. Not really ideal either, so I would stick with the current
approach with having multiple event channels.

Thanks!
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel