Re: [Xen-devel] [PATCH v2 07/10] vm_event: Add vm_event_ng interface

From: Petre Ovidiu PIRCALABU <ppircalabu@bitdefender.com>
To: Tamas K Lengyel <tamas@tklengyel.com>
Cc: "Stefano Stabellini" <sstabellini@kernel.org>,
	"Razvan Cojocaru" <rcojocaru@bitdefender.com>,
	"Wei Liu" <wl@xen.org>,
	"Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com>,
	"George Dunlap" <George.Dunlap@eu.citrix.com>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Ian Jackson" <ian.jackson@eu.citrix.com>,
	"Tim Deegan" <tim@xen.org>, "Julien Grall" <julien.grall@arm.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Alexandru Stefan ISAILA" <aisaila@bitdefender.com>,
	Xen-devel <xen-devel@lists.xenproject.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v2 07/10] vm_event: Add vm_event_ng interface
Date: Wed, 17 Jul 2019 12:13:49 +0000	[thread overview]
Message-ID: <52c8711c7bb1d4996012a611fdcb48cb4b9c3e5b.camel@bitdefender.com> (raw)
In-Reply-To: <CABfawhmwc1Be6Fdt_stEhn+ydZwiE1d-wX8d7WG+vXS2+C2d8g@mail.gmail.com>

On Tue, 2019-07-16 at 15:13 -0600, Tamas K Lengyel wrote:
> On Tue, Jul 16, 2019 at 11:06 AM Petre Pircalabu
> <ppircalabu@bitdefender.com> wrote:
> > 
> > In high throughput introspection scenarios where lots of monitor
> > vm_events are generated, the ring buffer can fill up before the
> > monitor
> > application gets a chance to handle all the requests thus blocking
> > other vcpus which will have to wait for a slot to become available.
> > 
> > This patch adds support for a different mechanism to handle
> > synchronous
> > vm_event requests / responses. As each synchronous request pauses
> > the
> > vcpu until the corresponding response is handled, it can be stored
> > in
> > a slotted memory buffer (one per vcpu) shared between the
> > hypervisor and
> > the controlling domain.
> > 
> > Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
> 
> So this is quite a large patch, perhaps it would be better to split
> it
> into a hypervisor-side patch and then a toolstack-side one. Also,
> would it make sense to keep the two implementations in separate files
> within Xen (ie. common/vm_event.c and vm_event_ng.c)?
> 
> Tamas
I thought about having 2 separate patches, one for hypervisor and one
for libxc, but my main concern was not to break "git bisect"(I'm not
sure if there are any tests (other than the build) which need to pass).
The "flags" field was added to vm_event_op, and, if it's not manually
set by the caller, an invalid value might be passed to XEN (e.g.
xc_vm_event_control).

Initially the new interface was contained in a separate file (has it's
own domctl) but I've copied it to vm_event because I wanted to have all
non-interface functions static. (e.g. the "enable" functions for both
transports and vm_event_handle_response). However, I will look into
this again, and, if I can find a clean way to minimize the
dependencies, I will split it again.

Many thanks,
Petre

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