Christoph Egger
2013-May-30 12:46 UTC
[PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Ian Campbell
2013-May-31 11:25 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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)
Frediano Ziglio
2013-Jun-11 13:46 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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)
>
>
Ian Campbell
2013-Jun-12 09:21 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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)
Frediano Ziglio
2013-Jun-12 12:02 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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)
>
>
Ian Campbell
2013-Jun-12 12:14 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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)
George Dunlap
2013-Jun-12 13:13 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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
Ian Campbell
2013-Jun-12 13:16 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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
Frediano Ziglio
2013-Jun-12 15:25 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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
Egger, Christoph
2013-Jun-13 09:47 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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) >
Ian Campbell
2013-Jun-27 11:31 UTC
Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD
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.