|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4/4] vm_event: Add support for multi-page ring buffer
On Thu, Sep 13, 2018 at 9:02 AM Petre Pircalabu
<ppircalabu@xxxxxxxxxxxxxxx> 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 extending the ring buffer by allocating a
> number of pages from domheap and mapping them to the monitor
> application's domain using the foreignmemory_map_resource interface.
> Unlike the current implementation, the ring buffer pages are not part of
> the introspected DomU, so they will not be reclaimed when the monitor is
> disabled.
>
> Signed-off-by: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
Thanks for this addition, it has been on the TODO for a long while
now. Could you also please push the patches as a git branch somewhere?
> ---
> tools/libxc/include/xenctrl.h | 2 +
> tools/libxc/xc_monitor.c | 7 +
> tools/libxc/xc_private.h | 3 +
> tools/libxc/xc_vm_event.c | 49 +++++++
> tools/tests/xen-access/xen-access.c | 33 +++--
> xen/arch/x86/domain_page.c | 3 +-
> xen/arch/x86/mm.c | 14 ++
> xen/common/vm_event.c | 258
> +++++++++++++++++++++++++++---------
> xen/include/public/domctl.h | 1 +
> xen/include/public/memory.h | 1 +
> xen/include/xen/sched.h | 5 +-
> xen/include/xen/vm_event.h | 4 +
> 12 files changed, 305 insertions(+), 75 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index bb75bcc..4f91ee9 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -2005,6 +2005,8 @@ int xc_get_mem_access(xc_interface *xch, uint32_t
> domain_id,
> * Caller has to unmap this page when done.
> */
> void *xc_monitor_enable(xc_interface *xch, uint32_t domain_id, uint32_t
> *port);
> +void *xc_monitor_enable_ex(xc_interface *xch, uint32_t domain_id,
> + int order, uint32_t *port);
I don't think there is value in keeping both version of the API
around. As libxc is not considered to be a stable API extending the
existing API would be the route I would prefer. Also, perhaps instead
of "order" name the input variable "page_count" to make it more clear
what it's about.
> int xc_monitor_disable(xc_interface *xch, uint32_t domain_id);
> int xc_monitor_resume(xc_interface *xch, uint32_t domain_id);
> /*
> diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
> index 15e6a0e..5188835 100644
> --- a/tools/libxc/xc_monitor.c
> +++ b/tools/libxc/xc_monitor.c
> @@ -28,6 +28,13 @@ void *xc_monitor_enable(xc_interface *xch, uint32_t
> domain_id, uint32_t *port)
> port);
> }
>
> +void *xc_monitor_enable_ex(xc_interface *xch, uint32_t domain_id, int order,
> + uint32_t *port)
> +{
> + return xc_vm_event_enable_ex(xch, domain_id, XEN_VM_EVENT_TYPE_MONITOR,
> + order, port);
> +}
> +
> int xc_monitor_disable(xc_interface *xch, uint32_t domain_id)
> {
> return xc_vm_event_control(xch, domain_id,
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index be22986..03d9460 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -436,6 +436,9 @@ int xc_vm_event_control(xc_interface *xch, uint32_t
> domain_id, unsigned int op,
> void *xc_vm_event_enable(xc_interface *xch, uint32_t domain_id, int type,
> uint32_t *port);
>
> +void *xc_vm_event_enable_ex(xc_interface *xch, uint32_t domain_id, int type,
> + int order, uint32_t *port);
> +
> int do_dm_op(xc_interface *xch, uint32_t domid, unsigned int nr_bufs, ...);
>
> #endif /* __XC_PRIVATE_H__ */
> diff --git a/tools/libxc/xc_vm_event.c b/tools/libxc/xc_vm_event.c
> index de37ca5..216bbe2 100644
> --- a/tools/libxc/xc_vm_event.c
> +++ b/tools/libxc/xc_vm_event.c
> @@ -21,6 +21,7 @@
> */
>
> #include "xc_private.h"
> +#include "xenforeignmemory.h"
>
> int xc_vm_event_control(xc_interface *xch, uint32_t domain_id, unsigned int
> op,
> unsigned int mode, uint32_t *port)
> @@ -184,6 +185,54 @@ void *xc_vm_event_enable(xc_interface *xch, uint32_t
> domain_id, int type,
> return ring_page;
> }
>
> +void *xc_vm_event_enable_ex(xc_interface *xch, uint32_t domain_id, int type,
> + int order, uint32_t *port)
> +{
> + xenforeignmemory_resource_handle *fres = NULL;
> + int saved_errno;
> + void *ring_buffer = NULL;
> +
> + if ( !port )
> + {
> + errno = EINVAL;
> + return NULL;
> + }
> +
> + /* Pause the domain for ring page setup */
> + if ( xc_domain_pause(xch, domain_id) )
> + {
> + PERROR("Unable to pause domain\n");
> + return NULL;
> + }
> +
> + fres = xenforeignmemory_map_resource(xch->fmem, domain_id,
> + XENMEM_resource_vm_event, type, 0,
> + order, &ring_buffer,
> + PROT_READ | PROT_WRITE, 0);
> + if ( !fres )
> + {
> + PERROR("Unable to map vm_event ring pages resource\n");
> + goto out;
> + }
> +
> + if ( xc_vm_event_control(xch, domain_id, XEN_VM_EVENT_GET_PORT, type,
> port) )
> + PERROR("Unable to get vm_event channel port\n");
> +
> +out:
> + saved_errno = errno;
> + if ( xc_domain_unpause(xch, domain_id) != 0 )
> + {
> + if (fres)
> + saved_errno = errno;
> + PERROR("Unable to unpause domain");
> + }
> +
> + free(fres);
> + errno = saved_errno;
> + return ring_buffer;
> +}
> +
> +
> /*
> * Local variables:
> * mode: C
> diff --git a/tools/tests/xen-access/xen-access.c
> b/tools/tests/xen-access/xen-access.c
> index 6aaee16..f4c4eda 100644
> --- a/tools/tests/xen-access/xen-access.c
> +++ b/tools/tests/xen-access/xen-access.c
> @@ -68,7 +68,8 @@ typedef struct vm_event {
> int port;
> vm_event_back_ring_t back_ring;
> uint32_t evtchn_port;
> - void *ring_page;
> + void *ring_buffer;
> + unsigned int ring_page_count;
> } vm_event_t;
>
> typedef struct xenaccess {
> @@ -136,8 +137,9 @@ int xenaccess_teardown(xc_interface *xch, xenaccess_t
> *xenaccess)
> return 0;
>
> /* Tear down domain xenaccess in Xen */
> - if ( xenaccess->vm_event.ring_page )
> - munmap(xenaccess->vm_event.ring_page, XC_PAGE_SIZE);
> + if ( xenaccess->vm_event.ring_buffer )
> + munmap(xenaccess->vm_event.ring_buffer,
> + xenaccess->vm_event.ring_page_count * XC_PAGE_SIZE );
>
> if ( mem_access_enable )
> {
> @@ -210,12 +212,25 @@ xenaccess_t *xenaccess_init(xc_interface **xch_r,
> domid_t domain_id)
> /* Set domain id */
> xenaccess->vm_event.domain_id = domain_id;
>
> - /* Enable mem_access */
> - xenaccess->vm_event.ring_page =
> + /* Ring buffer page count */
> + xenaccess->vm_event.ring_page_count = 2;
> +
> + xenaccess->vm_event.ring_buffer = xc_monitor_enable_ex(
> + xenaccess->xc_handle,
> + xenaccess->vm_event.domain_id,
> + xenaccess->vm_event.ring_page_count,
> + &xenaccess->vm_event.evtchn_port);
> +
> + if (xenaccess->vm_event.ring_buffer == NULL && errno == EOPNOTSUPP)
How would this situation ever arise? If there is a chance that you
can't setup multi-page rings, perhaps adding a hypercall that would
tell the user how many pages are max available for the ring is the
better route. This just seems like guessing right now.
> + {
> + xenaccess->vm_event.ring_page_count = 1;
> + xenaccess->vm_event.ring_buffer =
> xc_monitor_enable(xenaccess->xc_handle,
> xenaccess->vm_event.domain_id,
> &xenaccess->vm_event.evtchn_port);
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |