* [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD @ 2013-05-30 12:46 Christoph Egger 2013-05-31 11:25 ` Ian Campbell 0 siblings, 1 reply; 11+ messages in thread From: Christoph Egger @ 2013-05-30 12:46 UTC (permalink / raw) To: xen-devel; +Cc: Ian Campbell, Miguel Clara [-- Attachment #1: Type: text/plain, Size: 608 bytes --] tools/xencov: MAP_LOCKED is Linux specific. NetBSD's equivalent is MAP_WIRED. From: Miguel Clara <miguelmclara@gmail.com> Signed-off-by: Christoph Egger <chegger@amazon.de> diff -r f1fa13daf4b7 -r 97a4f7e36a8f tools/misc/xencov.c --- a/tools/misc/xencov.c Fri May 10 13:45:02 2013 +0000 +++ b/tools/misc/xencov.c Thu May 30 12:40:13 2013 +0000 @@ -26,6 +26,11 @@ #include <err.h> #include <sys/mman.h> +#ifdef __NetBSD__ +/* MAP_LOCKED is Linux specific. MAP_WIRED is NetBSD's equivalent. */ +#define MAP_LOCKED MAP_WIRED +#endif + static xc_interface *gcov_xch = NULL; static void gcov_init(void) [-- Attachment #2: patch_xencov.diff --] [-- Type: text/plain, Size: 794 bytes --] # HG changeset patch # User Christoph Egger <chegger@amazon.de> # Date 1369917613 0 # Node ID 97a4f7e36a8fb0bc37f8a6dc04a0e33b1bc9835d # Parent f1fa13daf4b78b027f081aca5be24cfa96813b42 tools/xencov: MAP_LOCKED is Linux specific. NetBSD's equivalent is MAP_WIRED. From: Miguel Clara <miguelmclara@gmail.com> Signed-off-by: Christoph Egger <chegger@amazon.de> diff -r f1fa13daf4b7 -r 97a4f7e36a8f tools/misc/xencov.c --- a/tools/misc/xencov.c Fri May 10 13:45:02 2013 +0000 +++ b/tools/misc/xencov.c Thu May 30 12:40:13 2013 +0000 @@ -26,6 +26,11 @@ #include <err.h> #include <sys/mman.h> +#ifdef __NetBSD__ +/* MAP_LOCKED is Linux specific. MAP_WIRED is NetBSD's equivalent. */ +#define MAP_LOCKED MAP_WIRED +#endif + static xc_interface *gcov_xch = NULL; static void gcov_init(void) [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-05-30 12:46 [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD Christoph Egger @ 2013-05-31 11:25 ` Ian Campbell 2013-06-11 13:46 ` Frediano Ziglio 0 siblings, 1 reply; 11+ messages in thread From: Ian Campbell @ 2013-05-31 11:25 UTC (permalink / raw) To: Christoph Egger; +Cc: Frediano Ziglio, Miguel Clara, xen-devel On Thu, 2013-05-30 at 14:46 +0200, Christoph Egger wrote: > tools/xencov: MAP_LOCKED is Linux specific. NetBSD's equivalent is MAP_WIRED. It looks to me as if xencov.c's shouldn't be using mmap in the first place. It should be using the libxc hypercall buffer code. I think xencov.c should be using xc_hypercall_buffer_alloc(_pages) and gcov_get_info should be xc_gcov_get_info & inside libxc not open coded in this tool. Not least because of its abuse of the GUEST_HANDLE interface (going directly at .p instead of using xen_set_guest_handle(), which is forbidden). Frediano, do you have time to sort this out? NB On Linux the hypercall buffers currently turn into MAP_PRIVATE| MAP_ANONYMOUS|MAP_LOCKED but on netbsd it is just xc_memalign. Presumably netbsd doesn't have to worry about the hypercall buffer being unmapped/swapped/paged/etc under its feet? Or if it does it needs to be fixed in libxc... Thanks, Ian. > From: Miguel Clara <miguelmclara@gmail.com> > Signed-off-by: Christoph Egger <chegger@amazon.de> > > diff -r f1fa13daf4b7 -r 97a4f7e36a8f tools/misc/xencov.c > --- a/tools/misc/xencov.c Fri May 10 13:45:02 2013 +0000 > +++ b/tools/misc/xencov.c Thu May 30 12:40:13 2013 +0000 > @@ -26,6 +26,11 @@ > #include <err.h> > #include <sys/mman.h> > > +#ifdef __NetBSD__ > +/* MAP_LOCKED is Linux specific. MAP_WIRED is NetBSD's equivalent. */ > +#define MAP_LOCKED MAP_WIRED > +#endif > + > static xc_interface *gcov_xch = NULL; > > static void gcov_init(void) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-05-31 11:25 ` Ian Campbell @ 2013-06-11 13:46 ` Frediano Ziglio 2013-06-12 9:21 ` Ian Campbell 0 siblings, 1 reply; 11+ messages in thread From: Frediano Ziglio @ 2013-06-11 13:46 UTC (permalink / raw) To: Ian Campbell; +Cc: Frediano Ziglio, Christoph Egger, Miguel Clara, xen-devel Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is more portable. Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> --- tools/misc/xencov.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) Here you are. It get tested on a x64 machine, if somebody could test it with a NetBSD would be great. diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c index 6645a30..e38d473 100644 --- a/tools/misc/xencov.c +++ b/tools/misc/xencov.c @@ -24,7 +24,6 @@ #include <unistd.h> #include <errno.h> #include <err.h> -#include <sys/mman.h> static xc_interface *gcov_xch = NULL; @@ -35,7 +34,7 @@ static void gcov_init(void) err(1, "opening interface"); } -int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) +int gcov_get_info(int op, struct xen_sysctl *sys, uint8_t *ptr) { struct xen_sysctl_coverage_op *cov; @@ -44,7 +43,7 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) cov = &sys->u.coverage_op; cov->cmd = op; - cov->u.raw_info.p = ptr; + set_xen_guest_handle_raw(cov->u.raw_info, ptr); return xc_sysctl(gcov_xch, sys); } @@ -52,10 +51,8 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) static void gcov_read(const char *fn, int reset) { struct xen_sysctl sys; - unsigned page_size = sysconf(_SC_PAGESIZE); uint32_t total_len; - uint8_t *p; - size_t size; + DECLARE_HYPERCALL_BUFFER(uint8_t, p); FILE *f; int op = reset ? XEN_SYSCTL_COVERAGE_read_and_reset : XEN_SYSCTL_COVERAGE_read; @@ -71,12 +68,9 @@ static void gcov_read(const char *fn, int reset) errx(1, "coverage size too big %u bytes\n", total_len); /* allocate */ - size = total_len + page_size; - size -= (size % page_size); - p = mmap(0, size, PROT_WRITE|PROT_READ, - MAP_PRIVATE|MAP_ANON|MAP_LOCKED, -1, 0); - if ( p == (uint8_t *) -1 ) - err(1, "mapping memory for coverage"); + p = xc_hypercall_buffer_alloc(gcov_xch, p, total_len); + if ( p == NULL ) + err(1, "allocating memory for coverage"); /* get data */ memset(p, 0, total_len); @@ -94,6 +88,7 @@ static void gcov_read(const char *fn, int reset) err(1, "writing coverage to file"); if (f != stdout) fclose(f); + xc_hypercall_buffer_free(gcov_xch, p); } static void gcov_reset(void) -- 1.7.10.4 On Fri, 2013-05-31 at 12:25 +0100, Ian Campbell wrote: > On Thu, 2013-05-30 at 14:46 +0200, Christoph Egger wrote: > > tools/xencov: MAP_LOCKED is Linux specific. NetBSD's equivalent is MAP_WIRED. > > It looks to me as if xencov.c's shouldn't be using mmap in the first > place. It should be using the libxc hypercall buffer code. > > I think xencov.c should be using xc_hypercall_buffer_alloc(_pages) and > gcov_get_info should be xc_gcov_get_info & inside libxc not open coded > in this tool. Not least because of its abuse of the GUEST_HANDLE > interface (going directly at .p instead of using xen_set_guest_handle(), > which is forbidden). > > Frediano, do you have time to sort this out? > > NB On Linux the hypercall buffers currently turn into MAP_PRIVATE| > MAP_ANONYMOUS|MAP_LOCKED but on netbsd it is just xc_memalign. > Presumably netbsd doesn't have to worry about the hypercall buffer being > unmapped/swapped/paged/etc under its feet? Or if it does it needs to be > fixed in libxc... > > Thanks, > Ian. > > > From: Miguel Clara <miguelmclara@gmail.com> > > Signed-off-by: Christoph Egger <chegger@amazon.de> > > > > diff -r f1fa13daf4b7 -r 97a4f7e36a8f tools/misc/xencov.c > > --- a/tools/misc/xencov.c Fri May 10 13:45:02 2013 +0000 > > +++ b/tools/misc/xencov.c Thu May 30 12:40:13 2013 +0000 > > @@ -26,6 +26,11 @@ > > #include <err.h> > > #include <sys/mman.h> > > > > +#ifdef __NetBSD__ > > +/* MAP_LOCKED is Linux specific. MAP_WIRED is NetBSD's equivalent. */ > > +#define MAP_LOCKED MAP_WIRED > > +#endif > > + > > static xc_interface *gcov_xch = NULL; > > > > static void gcov_init(void) > > ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-06-11 13:46 ` Frediano Ziglio @ 2013-06-12 9:21 ` Ian Campbell 2013-06-12 12:02 ` Frediano Ziglio 0 siblings, 1 reply; 11+ messages in thread From: Ian Campbell @ 2013-06-12 9:21 UTC (permalink / raw) To: Frediano Ziglio; +Cc: Frediano Ziglio, Christoph Egger, Miguel Clara, xen-devel On Tue, 2013-06-11 at 14:46 +0100, Frediano Ziglio wrote: > Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc > > xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is > more portable. > > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > --- > tools/misc/xencov.c | 19 +++++++------------ > 1 file changed, 7 insertions(+), 12 deletions(-) > > Here you are. It get tested on a x64 machine, if somebody could test it > with a NetBSD would be great. > > diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c > index 6645a30..e38d473 100644 > --- a/tools/misc/xencov.c > +++ b/tools/misc/xencov.c > @@ -24,7 +24,6 @@ > #include <unistd.h> > #include <errno.h> > #include <err.h> > -#include <sys/mman.h> > > static xc_interface *gcov_xch = NULL; > > @@ -35,7 +34,7 @@ static void gcov_init(void) > err(1, "opening interface"); > } > > -int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > +int gcov_get_info(int op, struct xen_sysctl *sys, uint8_t *ptr) > { > struct xen_sysctl_coverage_op *cov; > > @@ -44,7 +43,7 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > > cov = &sys->u.coverage_op; > cov->cmd = op; > - cov->u.raw_info.p = ptr; > + set_xen_guest_handle_raw(cov->u.raw_info, ptr); This isn't right I'm afraid. Your "void *ptr" here should become "struct xc_hypercall_buffer *ptr" and you should use DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr) in the body of the function and then drop the _raw from the set. The caller of this function then needs to pass HYPERCALL_BUFFER(name) to the function (where it looks like name == p in this case). xc_perfc_query() and its callers are an example of this pattern. Likewise xc_lockprof_query(). > > return xc_sysctl(gcov_xch, sys); > } > @@ -52,10 +51,8 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > static void gcov_read(const char *fn, int reset) > { > struct xen_sysctl sys; > - unsigned page_size = sysconf(_SC_PAGESIZE); > uint32_t total_len; > - uint8_t *p; > - size_t size; > + DECLARE_HYPERCALL_BUFFER(uint8_t, p); > FILE *f; > int op = reset ? XEN_SYSCTL_COVERAGE_read_and_reset : > XEN_SYSCTL_COVERAGE_read; > @@ -71,12 +68,9 @@ static void gcov_read(const char *fn, int reset) > errx(1, "coverage size too big %u bytes\n", total_len); > > /* allocate */ > - size = total_len + page_size; > - size -= (size % page_size); > - p = mmap(0, size, PROT_WRITE|PROT_READ, > - MAP_PRIVATE|MAP_ANON|MAP_LOCKED, -1, 0); > - if ( p == (uint8_t *) -1 ) > - err(1, "mapping memory for coverage"); > + p = xc_hypercall_buffer_alloc(gcov_xch, p, total_len); > + if ( p == NULL ) > + err(1, "allocating memory for coverage"); > > /* get data */ > memset(p, 0, total_len); > @@ -94,6 +88,7 @@ static void gcov_read(const char *fn, int reset) > err(1, "writing coverage to file"); > if (f != stdout) > fclose(f); > + xc_hypercall_buffer_free(gcov_xch, p); > } > > static void gcov_reset(void) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-06-12 9:21 ` Ian Campbell @ 2013-06-12 12:02 ` Frediano Ziglio 2013-06-12 12:14 ` Ian Campbell 2013-06-13 9:47 ` Egger, Christoph 0 siblings, 2 replies; 11+ messages in thread From: Frediano Ziglio @ 2013-06-12 12:02 UTC (permalink / raw) To: Ian Campbell; +Cc: Christoph Egger, Miguel Clara, Frediano Ziglio, xen-devel Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is more portable. Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> --- tools/misc/xencov.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) Here you are. It get tested on a x64 machine, if somebody could test it with a NetBSD would be great. diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c index 6645a30..fb4b2ff 100644 --- a/tools/misc/xencov.c +++ b/tools/misc/xencov.c @@ -24,7 +24,6 @@ #include <unistd.h> #include <errno.h> #include <err.h> -#include <sys/mman.h> static xc_interface *gcov_xch = NULL; @@ -35,16 +34,17 @@ static void gcov_init(void) err(1, "opening interface"); } -int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) +int gcov_get_info(int op, struct xen_sysctl *sys, struct xc_hypercall_buffer *ptr) { struct xen_sysctl_coverage_op *cov; + DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr); memset(sys, 0, sizeof(*sys)); sys->cmd = XEN_SYSCTL_coverage_op; cov = &sys->u.coverage_op; cov->cmd = op; - cov->u.raw_info.p = ptr; + set_xen_guest_handle(cov->u.raw_info, ptr); return xc_sysctl(gcov_xch, sys); } @@ -52,10 +52,8 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) static void gcov_read(const char *fn, int reset) { struct xen_sysctl sys; - unsigned page_size = sysconf(_SC_PAGESIZE); uint32_t total_len; - uint8_t *p; - size_t size; + DECLARE_HYPERCALL_BUFFER(uint8_t, p); FILE *f; int op = reset ? XEN_SYSCTL_COVERAGE_read_and_reset : XEN_SYSCTL_COVERAGE_read; @@ -71,16 +69,13 @@ static void gcov_read(const char *fn, int reset) errx(1, "coverage size too big %u bytes\n", total_len); /* allocate */ - size = total_len + page_size; - size -= (size % page_size); - p = mmap(0, size, PROT_WRITE|PROT_READ, - MAP_PRIVATE|MAP_ANON|MAP_LOCKED, -1, 0); - if ( p == (uint8_t *) -1 ) - err(1, "mapping memory for coverage"); + p = xc_hypercall_buffer_alloc(gcov_xch, p, total_len); + if ( p == NULL ) + err(1, "allocating memory for coverage"); /* get data */ memset(p, 0, total_len); - if ( gcov_get_info(op, &sys, p) < 0 ) + if ( gcov_get_info(op, &sys, HYPERCALL_BUFFER(p)) < 0 ) err(1, "getting coverage information"); /* write to a file */ @@ -94,6 +89,7 @@ static void gcov_read(const char *fn, int reset) err(1, "writing coverage to file"); if (f != stdout) fclose(f); + xc_hypercall_buffer_free(gcov_xch, p); } static void gcov_reset(void) -- 1.7.10.4 On Wed, 2013-06-12 at 10:21 +0100, Ian Campbell wrote: > On Tue, 2013-06-11 at 14:46 +0100, Frediano Ziglio wrote: > > Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc > > > > xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is > > more portable. > > > > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > > --- > > tools/misc/xencov.c | 19 +++++++------------ > > 1 file changed, 7 insertions(+), 12 deletions(-) > > > > Here you are. It get tested on a x64 machine, if somebody could test it > > with a NetBSD would be great. > > > > diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c > > index 6645a30..e38d473 100644 > > --- a/tools/misc/xencov.c > > +++ b/tools/misc/xencov.c > > @@ -24,7 +24,6 @@ > > #include <unistd.h> > > #include <errno.h> > > #include <err.h> > > -#include <sys/mman.h> > > > > static xc_interface *gcov_xch = NULL; > > > > @@ -35,7 +34,7 @@ static void gcov_init(void) > > err(1, "opening interface"); > > } > > > > -int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > > +int gcov_get_info(int op, struct xen_sysctl *sys, uint8_t *ptr) > > { > > struct xen_sysctl_coverage_op *cov; > > > > @@ -44,7 +43,7 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > > > > cov = &sys->u.coverage_op; > > cov->cmd = op; > > - cov->u.raw_info.p = ptr; > > + set_xen_guest_handle_raw(cov->u.raw_info, ptr); > > This isn't right I'm afraid. Your "void *ptr" here should become "struct > xc_hypercall_buffer *ptr" and you should use > DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr) in the body of the function and > then drop the _raw from the set. > > The caller of this function then needs to pass HYPERCALL_BUFFER(name) to > the function (where it looks like name == p in this case). > > xc_perfc_query() and its callers are an example of this pattern. > Likewise xc_lockprof_query(). > > > > > return xc_sysctl(gcov_xch, sys); > > } > > @@ -52,10 +51,8 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > > static void gcov_read(const char *fn, int reset) > > { > > struct xen_sysctl sys; > > - unsigned page_size = sysconf(_SC_PAGESIZE); > > uint32_t total_len; > > - uint8_t *p; > > - size_t size; > > + DECLARE_HYPERCALL_BUFFER(uint8_t, p); > > FILE *f; > > int op = reset ? XEN_SYSCTL_COVERAGE_read_and_reset : > > XEN_SYSCTL_COVERAGE_read; > > @@ -71,12 +68,9 @@ static void gcov_read(const char *fn, int reset) > > errx(1, "coverage size too big %u bytes\n", total_len); > > > > /* allocate */ > > - size = total_len + page_size; > > - size -= (size % page_size); > > - p = mmap(0, size, PROT_WRITE|PROT_READ, > > - MAP_PRIVATE|MAP_ANON|MAP_LOCKED, -1, 0); > > - if ( p == (uint8_t *) -1 ) > > - err(1, "mapping memory for coverage"); > > + p = xc_hypercall_buffer_alloc(gcov_xch, p, total_len); > > + if ( p == NULL ) > > + err(1, "allocating memory for coverage"); > > > > /* get data */ > > memset(p, 0, total_len); > > @@ -94,6 +88,7 @@ static void gcov_read(const char *fn, int reset) > > err(1, "writing coverage to file"); > > if (f != stdout) > > fclose(f); > > + xc_hypercall_buffer_free(gcov_xch, p); > > } > > > > static void gcov_reset(void) > > ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-06-12 12:02 ` Frediano Ziglio @ 2013-06-12 12:14 ` Ian Campbell 2013-06-12 13:13 ` George Dunlap 2013-06-13 9:47 ` Egger, Christoph 1 sibling, 1 reply; 11+ messages in thread From: Ian Campbell @ 2013-06-12 12:14 UTC (permalink / raw) To: Frediano Ziglio Cc: George Dunlap, Christoph Egger, Miguel Clara, Frediano Ziglio, xen-devel On Wed, 2013-06-12 at 13:02 +0100, Frediano Ziglio wrote: > Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc > > xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is > more portable. > > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> Looks good to me, thanks, Acked-by: Ian Campbell <ian.campbell@citrix.com> George, what is your feeling for the release? This is a somewhat obscure tool and IMHO the risk of regressing is small. The change fixes the default build on NetBSD. > --- > tools/misc/xencov.c | 22 +++++++++------------- > 1 file changed, 9 insertions(+), 13 deletions(-) > > Here you are. It get tested on a x64 machine, if somebody could test it > with a NetBSD would be great. That would certainly be useful and appreciated although since it currently doesn't build on NetBSD there is not much chance of regressing there so I'll probably apply (subject to George's approval) in a day or so regardless (unless the test is done and fails of course). > > diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c > index 6645a30..fb4b2ff 100644 > --- a/tools/misc/xencov.c > +++ b/tools/misc/xencov.c > @@ -24,7 +24,6 @@ > #include <unistd.h> > #include <errno.h> > #include <err.h> > -#include <sys/mman.h> > > static xc_interface *gcov_xch = NULL; > > @@ -35,16 +34,17 @@ static void gcov_init(void) > err(1, "opening interface"); > } > > -int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > +int gcov_get_info(int op, struct xen_sysctl *sys, struct xc_hypercall_buffer *ptr) > { > struct xen_sysctl_coverage_op *cov; > + DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr); > > memset(sys, 0, sizeof(*sys)); > sys->cmd = XEN_SYSCTL_coverage_op; > > cov = &sys->u.coverage_op; > cov->cmd = op; > - cov->u.raw_info.p = ptr; > + set_xen_guest_handle(cov->u.raw_info, ptr); > > return xc_sysctl(gcov_xch, sys); > } > @@ -52,10 +52,8 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > static void gcov_read(const char *fn, int reset) > { > struct xen_sysctl sys; > - unsigned page_size = sysconf(_SC_PAGESIZE); > uint32_t total_len; > - uint8_t *p; > - size_t size; > + DECLARE_HYPERCALL_BUFFER(uint8_t, p); > FILE *f; > int op = reset ? XEN_SYSCTL_COVERAGE_read_and_reset : > XEN_SYSCTL_COVERAGE_read; > @@ -71,16 +69,13 @@ static void gcov_read(const char *fn, int reset) > errx(1, "coverage size too big %u bytes\n", total_len); > > /* allocate */ > - size = total_len + page_size; > - size -= (size % page_size); > - p = mmap(0, size, PROT_WRITE|PROT_READ, > - MAP_PRIVATE|MAP_ANON|MAP_LOCKED, -1, 0); > - if ( p == (uint8_t *) -1 ) > - err(1, "mapping memory for coverage"); > + p = xc_hypercall_buffer_alloc(gcov_xch, p, total_len); > + if ( p == NULL ) > + err(1, "allocating memory for coverage"); > > /* get data */ > memset(p, 0, total_len); > - if ( gcov_get_info(op, &sys, p) < 0 ) > + if ( gcov_get_info(op, &sys, HYPERCALL_BUFFER(p)) < 0 ) > err(1, "getting coverage information"); > > /* write to a file */ > @@ -94,6 +89,7 @@ static void gcov_read(const char *fn, int reset) > err(1, "writing coverage to file"); > if (f != stdout) > fclose(f); > + xc_hypercall_buffer_free(gcov_xch, p); > } > > static void gcov_reset(void) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-06-12 12:14 ` Ian Campbell @ 2013-06-12 13:13 ` George Dunlap 2013-06-12 13:16 ` Ian Campbell ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: George Dunlap @ 2013-06-12 13:13 UTC (permalink / raw) To: Ian Campbell Cc: Frediano Ziglio, Christoph Egger, Miguel Clara, Frediano Ziglio, xen-devel On 12/06/13 13:14, Ian Campbell wrote: > On Wed, 2013-06-12 at 13:02 +0100, Frediano Ziglio wrote: >> Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc >> >> xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is >> more portable. >> >> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > Looks good to me, thanks, > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > George, what is your feeling for the release? This is a somewhat obscure > tool and IMHO the risk of regressing is small. The change fixes the > default build on NetBSD. Did we even have gcov in 4.2? Anyway I agree, the benefits are worth more than the risk. Re the release: Acked-by: George Dunlap <george.dunlap@eu.citrix.com> -George ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-06-12 13:13 ` George Dunlap @ 2013-06-12 13:16 ` Ian Campbell 2013-06-12 15:25 ` Frediano Ziglio 2013-06-27 11:31 ` Ian Campbell 2 siblings, 0 replies; 11+ messages in thread From: Ian Campbell @ 2013-06-12 13:16 UTC (permalink / raw) To: George Dunlap Cc: Frediano Ziglio, Christoph Egger, Frediano Ziglio, Miguel Clara, xen-devel On Wed, 2013-06-12 at 14:13 +0100, George Dunlap wrote: > On 12/06/13 13:14, Ian Campbell wrote: > > On Wed, 2013-06-12 at 13:02 +0100, Frediano Ziglio wrote: > >> Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc > >> > >> xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is > >> more portable. > >> > >> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > > Looks good to me, thanks, > > > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > > > George, what is your feeling for the release? This is a somewhat obscure > > tool and IMHO the risk of regressing is small. The change fixes the > > default build on NetBSD. > > Did we even have gcov in 4.2? Good point, no we didn't. > Anyway I agree, the benefits are worth > more than the risk. > > Re the release: > Acked-by: George Dunlap <george.dunlap@eu.citrix.com> > > -George > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-06-12 13:13 ` George Dunlap 2013-06-12 13:16 ` Ian Campbell @ 2013-06-12 15:25 ` Frediano Ziglio 2013-06-27 11:31 ` Ian Campbell 2 siblings, 0 replies; 11+ messages in thread From: Frediano Ziglio @ 2013-06-12 15:25 UTC (permalink / raw) To: George Dunlap Cc: Miguel Clara, Christoph Egger, Ian Campbell, Frediano Ziglio, xen-devel On Wed, 2013-06-12 at 14:13 +0100, George Dunlap wrote: > On 12/06/13 13:14, Ian Campbell wrote: > > On Wed, 2013-06-12 at 13:02 +0100, Frediano Ziglio wrote: > >> Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc > >> > >> xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is > >> more portable. > >> > >> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > > Looks good to me, thanks, > > > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > > > George, what is your feeling for the release? This is a somewhat obscure > > tool and IMHO the risk of regressing is small. The change fixes the > > default build on NetBSD. > > Did we even have gcov in 4.2? Anyway I agree, the benefits are worth > more than the risk. > No, was introduced for 4.3. > Re the release: > Acked-by: George Dunlap <george.dunlap@eu.citrix.com> > > -George > Frediano ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-06-12 13:13 ` George Dunlap 2013-06-12 13:16 ` Ian Campbell 2013-06-12 15:25 ` Frediano Ziglio @ 2013-06-27 11:31 ` Ian Campbell 2 siblings, 0 replies; 11+ messages in thread From: Ian Campbell @ 2013-06-27 11:31 UTC (permalink / raw) To: George Dunlap Cc: Frediano Ziglio, Christoph Egger, Frediano Ziglio, Miguel Clara, xen-devel On Wed, 2013-06-12 at 14:13 +0100, George Dunlap wrote: > On 12/06/13 13:14, Ian Campbell wrote: > > On Wed, 2013-06-12 at 13:02 +0100, Frediano Ziglio wrote: > >> Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc > >> > >> xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is > >> more portable. > >> > >> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> > > Looks good to me, thanks, > > > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > > > George, what is your feeling for the release? This is a somewhat obscure > > tool and IMHO the risk of regressing is small. The change fixes the > > default build on NetBSD. > > Did we even have gcov in 4.2? Anyway I agree, the benefits are worth > more than the risk. > > Re the release: > Acked-by: George Dunlap <george.dunlap@eu.citrix.com> This was a little while ago, but I've committed. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD 2013-06-12 12:02 ` Frediano Ziglio 2013-06-12 12:14 ` Ian Campbell @ 2013-06-13 9:47 ` Egger, Christoph 1 sibling, 0 replies; 11+ messages in thread From: Egger, Christoph @ 2013-06-13 9:47 UTC (permalink / raw) To: Frediano Ziglio; +Cc: Miguel Clara, Ian Campbell, Frediano Ziglio, xen-devel On 12.06.13 14:02, Frediano Ziglio wrote: > Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc > > xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is > more portable. > > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com> Acked-by: Christoph Egger <chegger@amazon.de> > --- > tools/misc/xencov.c | 22 +++++++++------------- > 1 file changed, 9 insertions(+), 13 deletions(-) > > Here you are. It get tested on a x64 machine, if somebody could test it > with a NetBSD would be great. > > diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c > index 6645a30..fb4b2ff 100644 > --- a/tools/misc/xencov.c > +++ b/tools/misc/xencov.c > @@ -24,7 +24,6 @@ > #include <unistd.h> > #include <errno.h> > #include <err.h> > -#include <sys/mman.h> > > static xc_interface *gcov_xch = NULL; > > @@ -35,16 +34,17 @@ static void gcov_init(void) > err(1, "opening interface"); > } > > -int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > +int gcov_get_info(int op, struct xen_sysctl *sys, struct xc_hypercall_buffer *ptr) > { > struct xen_sysctl_coverage_op *cov; > + DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr); > > memset(sys, 0, sizeof(*sys)); > sys->cmd = XEN_SYSCTL_coverage_op; > > cov = &sys->u.coverage_op; > cov->cmd = op; > - cov->u.raw_info.p = ptr; > + set_xen_guest_handle(cov->u.raw_info, ptr); > > return xc_sysctl(gcov_xch, sys); > } > @@ -52,10 +52,8 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr) > static void gcov_read(const char *fn, int reset) > { > struct xen_sysctl sys; > - unsigned page_size = sysconf(_SC_PAGESIZE); > uint32_t total_len; > - uint8_t *p; > - size_t size; > + DECLARE_HYPERCALL_BUFFER(uint8_t, p); > FILE *f; > int op = reset ? XEN_SYSCTL_COVERAGE_read_and_reset : > XEN_SYSCTL_COVERAGE_read; > @@ -71,16 +69,13 @@ static void gcov_read(const char *fn, int reset) > errx(1, "coverage size too big %u bytes\n", total_len); > > /* allocate */ > - size = total_len + page_size; > - size -= (size % page_size); > - p = mmap(0, size, PROT_WRITE|PROT_READ, > - MAP_PRIVATE|MAP_ANON|MAP_LOCKED, -1, 0); > - if ( p == (uint8_t *) -1 ) > - err(1, "mapping memory for coverage"); > + p = xc_hypercall_buffer_alloc(gcov_xch, p, total_len); > + if ( p == NULL ) > + err(1, "allocating memory for coverage"); > > /* get data */ > memset(p, 0, total_len); > - if ( gcov_get_info(op, &sys, p) < 0 ) > + if ( gcov_get_info(op, &sys, HYPERCALL_BUFFER(p)) < 0 ) > err(1, "getting coverage information"); > > /* write to a file */ > @@ -94,6 +89,7 @@ static void gcov_read(const char *fn, int reset) > err(1, "writing coverage to file"); > if (f != stdout) > fclose(f); > + xc_hypercall_buffer_free(gcov_xch, p); > } > > static void gcov_reset(void) > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-06-27 11:31 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-05-30 12:46 [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD Christoph Egger 2013-05-31 11:25 ` Ian Campbell 2013-06-11 13:46 ` Frediano Ziglio 2013-06-12 9:21 ` Ian Campbell 2013-06-12 12:02 ` Frediano Ziglio 2013-06-12 12:14 ` Ian Campbell 2013-06-12 13:13 ` George Dunlap 2013-06-12 13:16 ` Ian Campbell 2013-06-12 15:25 ` Frediano Ziglio 2013-06-27 11:31 ` Ian Campbell 2013-06-13 9:47 ` Egger, Christoph
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.