* xenpm fail
@ 2010-10-29 8:32 Zhang, Yang Z
2010-10-29 8:55 ` Ian Campbell
2010-10-29 9:26 ` Ian Campbell
0 siblings, 2 replies; 11+ messages in thread
From: Zhang, Yang Z @ 2010-10-29 8:32 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, ian.campbell
Hi ian
I find Cs 22292 cause xenpm broken. When run "xenpm start" or "xenpm get-cpuidle-states" and other xenmpm command, it will get segment fault.
After do some investigation, I find call xc_pm_get_cxstat() will free the cxstat->tiggers,
For example:
Here is some code form my test.c.
struct xc_cx_stat cxstatinfo, *cxstat = &cxstatinfo;
cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
if ( !cxstat->triggers ) {
printf("get memory fail");
return NOMEM;
}
ret = xc_pm_get_cxstat(xc_handle, cpu, cxstat);
printf("triggers=%lx \n", cxstat->triggers[0]);
Run it, and it will show segment fault at print the cxtat->tiggers[0]. It seems that xc_pm_get_cxstat() will free cxstat->triggers which we allocate memory before, and then when try to touch cxstat->tiggers[0], the issue raised.
If remove the patch 22292, everything is ok.
best regards
yang
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: xenpm fail
2010-10-29 8:32 xenpm fail Zhang, Yang Z
@ 2010-10-29 8:55 ` Ian Campbell
2010-10-29 9:26 ` Ian Campbell
1 sibling, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2010-10-29 8:55 UTC (permalink / raw)
To: Zhang, Yang Z; +Cc: xen-devel, Ian Jackson
Thanks for the report. I can't reproduce on first attempt but I will
take a look.
On Fri, 2010-10-29 at 09:32 +0100, Zhang, Yang Z wrote:
> Hi ian
> I find Cs 22292 cause xenpm broken. When run "xenpm start" or "xenpm get-cpuidle-states" and other xenmpm command, it will get segment fault.
> After do some investigation, I find call xc_pm_get_cxstat() will free the cxstat->tiggers,
> For example:
> Here is some code form my test.c.
>
> struct xc_cx_stat cxstatinfo, *cxstat = &cxstatinfo;
>
> cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
>
> if ( !cxstat->triggers ) {
> printf("get memory fail");
> return NOMEM;
> }
> ret = xc_pm_get_cxstat(xc_handle, cpu, cxstat);
> printf("triggers=%lx \n", cxstat->triggers[0]);
>
> Run it, and it will show segment fault at print the cxtat->tiggers[0]. It seems that xc_pm_get_cxstat() will free cxstat->triggers which we allocate memory before, and then when try to touch cxstat->tiggers[0], the issue raised.
> If remove the patch 22292, everything is ok.
>
> best regards
> yang
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: xenpm fail
2010-10-29 8:32 xenpm fail Zhang, Yang Z
2010-10-29 8:55 ` Ian Campbell
@ 2010-10-29 9:26 ` Ian Campbell
2010-10-29 9:42 ` Ian Campbell
2010-11-01 2:14 ` Zhang, Yang Z
1 sibling, 2 replies; 11+ messages in thread
From: Ian Campbell @ 2010-10-29 9:26 UTC (permalink / raw)
To: Zhang, Yang Z; +Cc: xen-devel, Ian Jackson
On Fri, 2010-10-29 at 09:32 +0100, Zhang, Yang Z wrote:
> Hi ian
> I find Cs 22292 cause xenpm broken. When run "xenpm start" or "xenpm
> get-cpuidle-states" and other xenmpm command, it will get segment
> fault.
> After do some investigation, I find call xc_pm_get_cxstat() will free
> the cxstat->tiggers,
> For example:
> Here is some code form my test.c.
>
> struct xc_cx_stat cxstatinfo, *cxstat = &cxstatinfo;
>
> cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
>
> if ( !cxstat->triggers ) {
> printf("get memory fail");
> return NOMEM;
> }
> ret = xc_pm_get_cxstat(xc_handle, cpu, cxstat);
what is ret at this point?
> printf("triggers=%lx \n", cxstat->triggers[0]);
>
> Run it, and it will show segment fault at print the cxtat->tiggers[0].
> It seems that xc_pm_get_cxstat() will free cxstat->triggers which we
> allocate memory before, and then when try to touch cxstat->tiggers[0],
> the issue raised.
I can't see any code which frees cxstat->triggers in xc_pm_get_cxstat,
there is only code which frees the bounce buffer.
Perhaps the issue you are seeing is with get_cxstat_by_cpuid from
xenpm.c rather than xc_pm_get_cxstat directly? I notice that
get_cxstat_by_cpuid is called on one occasion without checking the
return code and that it frees the trigger array when xc_pm_get_cxstat
fails so a new failure introduced by 22292 could cause this?
What hardware is this on? What is max_cx_num and max_cpu_nr for you?
> If remove the patch 22292, everything is ok.
>
> best regards
> yang
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Re: xenpm fail
2010-10-29 9:26 ` Ian Campbell
@ 2010-10-29 9:42 ` Ian Campbell
2010-11-01 2:17 ` Zhang, Yang Z
2010-11-01 2:14 ` Zhang, Yang Z
1 sibling, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2010-10-29 9:42 UTC (permalink / raw)
To: Zhang, Yang Z; +Cc: xen-devel, Ian Jackson
On Fri, 2010-10-29 at 10:26 +0100, Ian Campbell wrote:
> On Fri, 2010-10-29 at 09:32 +0100, Zhang, Yang Z wrote:
> > Hi ian
> > I find Cs 22292 cause xenpm broken. When run "xenpm start" or "xenpm
> > get-cpuidle-states" and other xenmpm command, it will get segment
> > fault.
> > After do some investigation, I find call xc_pm_get_cxstat() will free
> > the cxstat->tiggers,
> > For example:
> > Here is some code form my test.c.
> >
> > struct xc_cx_stat cxstatinfo, *cxstat = &cxstatinfo;
> >
> > cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
> >
> > if ( !cxstat->triggers ) {
> > printf("get memory fail");
> > return NOMEM;
> > }
> > ret = xc_pm_get_cxstat(xc_handle, cpu, cxstat);
>
> what is ret at this point?
>
> > printf("triggers=%lx \n", cxstat->triggers[0]);
> >
> > Run it, and it will show segment fault at print the cxtat->tiggers[0].
> > It seems that xc_pm_get_cxstat() will free cxstat->triggers which we
> > allocate memory before, and then when try to touch cxstat->tiggers[0],
> > the issue raised.
>
> I can't see any code which frees cxstat->triggers in xc_pm_get_cxstat,
> there is only code which frees the bounce buffer.
>
> Perhaps the issue you are seeing is with get_cxstat_by_cpuid from
> xenpm.c rather than xc_pm_get_cxstat directly? I notice that
> get_cxstat_by_cpuid is called on one occasion without checking the
> return code and that it frees the trigger array when xc_pm_get_cxstat
> fails so a new failure introduced by 22292 could cause this?
>
> What hardware is this on? What is max_cx_num and max_cpu_nr for you?
Please could you also try this debug patch
diff -r a1b39d2b9001 tools/misc/xenpm.c
--- a/tools/misc/xenpm.c Fri Oct 22 15:14:51 2010 +0100
+++ b/tools/misc/xenpm.c Fri Oct 29 10:41:37 2010 +0100
@@ -121,6 +121,7 @@ static int get_cxstat_by_cpuid(xc_interf
cxstat->residencies = malloc(max_cx_num * sizeof(uint64_t));
if ( !cxstat->residencies )
{
+ fprintf(stderr, "failed to allocate residencies, freeing triggers\n");
free(cxstat->triggers);
return -ENOMEM;
}
@@ -129,6 +130,7 @@ static int get_cxstat_by_cpuid(xc_interf
if( ret )
{
int temp = errno;
+ fprintf(stderr, "xc_pm_get_cx_stat failed %d %d, freeing buffers\n", ret, errno);
free(cxstat->triggers);
free(cxstat->residencies);
cxstat->triggers = NULL;
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: xenpm fail
2010-10-29 9:26 ` Ian Campbell
2010-10-29 9:42 ` Ian Campbell
@ 2010-11-01 2:14 ` Zhang, Yang Z
2010-11-01 10:26 ` Ian Campbell
1 sibling, 1 reply; 11+ messages in thread
From: Zhang, Yang Z @ 2010-11-01 2:14 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian, xen-devel, Jackson
[-- Attachment #1: Type: text/plain, Size: 2004 bytes --]
> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@eu.citrix.com]
> Sent: Friday, October 29, 2010 5:26 PM
> To: Zhang, Yang Z
> Cc: xen-devel@lists.xensource.com; Ian Jackson
> Subject: Re: xenpm fail
>
> On Fri, 2010-10-29 at 09:32 +0100, Zhang, Yang Z wrote:
> > Hi ian
> > I find Cs 22292 cause xenpm broken. When run "xenpm start" or "xenpm
> > get-cpuidle-states" and other xenmpm command, it will get segment
> > fault.
> > After do some investigation, I find call xc_pm_get_cxstat() will free
> > the cxstat->tiggers,
> > For example:
> > Here is some code form my test.c.
> >
> > struct xc_cx_stat cxstatinfo, *cxstat = &cxstatinfo;
> >
> > cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
> >
> > if ( !cxstat->triggers ) {
> > printf("get memory fail");
> > return NOMEM;
> > }
> > ret = xc_pm_get_cxstat(xc_handle, cpu, cxstat);
>
> what is ret at this point?
>
ret = 0
> > printf("triggers=%lx \n", cxstat->triggers[0]);
> >
> > Run it, and it will show segment fault at print the cxtat->tiggers[0].
> > It seems that xc_pm_get_cxstat() will free cxstat->triggers which we
> > allocate memory before, and then when try to touch cxstat->tiggers[0],
> > the issue raised.
>
> I can't see any code which frees cxstat->triggers in xc_pm_get_cxstat,
> there is only code which frees the bounce buffer.
>
> Perhaps the issue you are seeing is with get_cxstat_by_cpuid from
> xenpm.c rather than xc_pm_get_cxstat directly? I notice that
> get_cxstat_by_cpuid is called on one occasion without checking the
> return code and that it frees the trigger array when xc_pm_get_cxstat
> fails so a new failure introduced by 22292 could cause this?
>
> What hardware is this on? What is max_cx_num and max_cpu_nr for you?
>
I used westmere-ep. max_cx_num equal to 4 and I didn't get the max_cpu_nr in my test case.
> > If remove the patch 22292, everything is ok.
> >
> > best regards
> > yang
> >
> >
>
[-- Attachment #2: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: Re: xenpm fail
2010-10-29 9:42 ` Ian Campbell
@ 2010-11-01 2:17 ` Zhang, Yang Z
0 siblings, 0 replies; 11+ messages in thread
From: Zhang, Yang Z @ 2010-11-01 2:17 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian, xen-devel, Jackson
[-- Attachment #1: Type: text/plain, Size: 2885 bytes --]
Nothing print with your patch.
best regards
yang
> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@citrix.com]
> Sent: Friday, October 29, 2010 5:42 PM
> To: Zhang, Yang Z
> Cc: xen-devel@lists.xensource.com; Ian Jackson
> Subject: Re: [Xen-devel] Re: xenpm fail
>
> On Fri, 2010-10-29 at 10:26 +0100, Ian Campbell wrote:
> > On Fri, 2010-10-29 at 09:32 +0100, Zhang, Yang Z wrote:
> > > Hi ian
> > > I find Cs 22292 cause xenpm broken. When run "xenpm start" or "xenpm
> > > get-cpuidle-states" and other xenmpm command, it will get segment
> > > fault.
> > > After do some investigation, I find call xc_pm_get_cxstat() will free
> > > the cxstat->tiggers,
> > > For example:
> > > Here is some code form my test.c.
> > >
> > > struct xc_cx_stat cxstatinfo, *cxstat = &cxstatinfo;
> > >
> > > cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
> > >
> > > if ( !cxstat->triggers ) {
> > > printf("get memory fail");
> > > return NOMEM;
> > > }
> > > ret = xc_pm_get_cxstat(xc_handle, cpu, cxstat);
> >
> > what is ret at this point?
> >
> > > printf("triggers=%lx \n", cxstat->triggers[0]);
> > >
> > > Run it, and it will show segment fault at print the cxtat->tiggers[0].
> > > It seems that xc_pm_get_cxstat() will free cxstat->triggers which we
> > > allocate memory before, and then when try to touch cxstat->tiggers[0],
> > > the issue raised.
> >
> > I can't see any code which frees cxstat->triggers in xc_pm_get_cxstat,
> > there is only code which frees the bounce buffer.
> >
> > Perhaps the issue you are seeing is with get_cxstat_by_cpuid from
> > xenpm.c rather than xc_pm_get_cxstat directly? I notice that
> > get_cxstat_by_cpuid is called on one occasion without checking the
> > return code and that it frees the trigger array when xc_pm_get_cxstat
> > fails so a new failure introduced by 22292 could cause this?
> >
> > What hardware is this on? What is max_cx_num and max_cpu_nr for you?
>
> Please could you also try this debug patch
>
> diff -r a1b39d2b9001 tools/misc/xenpm.c
> --- a/tools/misc/xenpm.c Fri Oct 22 15:14:51 2010 +0100
> +++ b/tools/misc/xenpm.c Fri Oct 29 10:41:37 2010 +0100
> @@ -121,6 +121,7 @@ static int get_cxstat_by_cpuid(xc_interf
> cxstat->residencies = malloc(max_cx_num * sizeof(uint64_t));
> if ( !cxstat->residencies )
> {
> + fprintf(stderr, "failed to allocate residencies, freeing triggers\n");
> free(cxstat->triggers);
> return -ENOMEM;
> }
> @@ -129,6 +130,7 @@ static int get_cxstat_by_cpuid(xc_interf
> if( ret )
> {
> int temp = errno;
> + fprintf(stderr, "xc_pm_get_cx_stat failed %d %d, freeing buffers\n",
> ret, errno);
> free(cxstat->triggers);
> free(cxstat->residencies);
> cxstat->triggers = NULL;
>
[-- Attachment #2: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: xenpm fail
2010-11-01 2:14 ` Zhang, Yang Z
@ 2010-11-01 10:26 ` Ian Campbell
2010-11-01 10:49 ` Zhang, Yang Z
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2010-11-01 10:26 UTC (permalink / raw)
To: Zhang, Yang Z; +Cc: xen-devel, Ian Jackson
[-- Attachment #1: Type: text/plain, Size: 1832 bytes --]
On Mon, 2010-11-01 at 02:14 +0000, Zhang, Yang Z wrote:
> > -----Original Message-----
> > From: Ian Campbell [mailto:Ian.Campbell@eu.citrix.com]
> > Sent: Friday, October 29, 2010 5:26 PM
> > To: Zhang, Yang Z
> > Cc: xen-devel@lists.xensource.com; Ian Jackson
> > Subject: Re: xenpm fail
> >
> > On Fri, 2010-10-29 at 09:32 +0100, Zhang, Yang Z wrote:
> > > Hi ian
> > > I find Cs 22292 cause xenpm broken. When run "xenpm start" or "xenpm
> > > get-cpuidle-states" and other xenmpm command, it will get segment
> > > fault.
> > > After do some investigation, I find call xc_pm_get_cxstat() will free
> > > the cxstat->tiggers,
> > > For example:
> > > Here is some code form my test.c.
> > >
> > > struct xc_cx_stat cxstatinfo, *cxstat = &cxstatinfo;
> > >
> > > cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
> > >
> > > if ( !cxstat->triggers ) {
> > > printf("get memory fail");
> > > return NOMEM;
> > > }
> > > ret = xc_pm_get_cxstat(xc_handle, cpu, cxstat);
> >
> > what is ret at this point?
> >
> ret = 0
Are you running the precise code you give above? xc_pm_get_cxstat will
return failure if cxstat->residencies is not initialised. This didn't
change in 22292:a1b39d2b9001. I suspect the error you are seeing with
your test.c may be due to this and unrelated to the problem(s) with
xenpm.
My guess is that 22292:a1b39d2b9001 added a new potential failure case
to xc_pm_get_cxstat (the bounce buffer allocation) which is causing an
error to be returned which is incorrectly handled in xenpm but
unfortunately I can't see it from staring at the code.
Please can you try the attached, increasingly desperate, debugging patch
and send the complete output of running both your test case and xenpm.
Please could you also run xenpm under gdb and grab a backtrace from the
location of the segfault.
Ian.
[-- Attachment #2: xc_pm-debug.patch --]
[-- Type: text/x-patch, Size: 4567 bytes --]
diff -r 86640f3de07f tools/libxc/xc_hcall_buf.c
--- a/tools/libxc/xc_hcall_buf.c Mon Nov 01 10:03:55 2010 +0000
+++ b/tools/libxc/xc_hcall_buf.c Mon Nov 01 10:21:05 2010 +0000
@@ -120,17 +120,22 @@ int xc__hypercall_bounce_pre(xc_interfac
*/
if ( b->ubuf == NULL )
{
+ fprintf(stderr, "%s user buffer is NULL at %d\n", __func__, __LINE__);
b->hbuf = NULL;
return 0;
}
p = xc__hypercall_buffer_alloc(xch, b, b->sz);
if ( p == NULL )
+ {
+ fprintf(stderr, "%s failed at %d\n", __func__, __LINE__);
return -1;
+ }
if ( b->dir == XC_HYPERCALL_BUFFER_BOUNCE_IN || b->dir == XC_HYPERCALL_BUFFER_BOUNCE_BOTH )
memcpy(b->hbuf, b->ubuf, b->sz);
+ fprintf(stderr, "%s bounced %zd bytes from user buf %p into hcall buf %p\n", __func__, b->sz, b->ubuf, b->hbuf);
return 0;
}
@@ -143,11 +148,15 @@ void xc__hypercall_bounce_post(xc_interf
abort();
if ( b->hbuf == NULL )
+ {
+ fprintf(stderr, "%s hcall buffer is NULL at %d (%p, %p)\n", __func__, __LINE__, b->hbuf, b->ubuf);
return;
+ }
if ( b->dir == XC_HYPERCALL_BUFFER_BOUNCE_OUT || b->dir == XC_HYPERCALL_BUFFER_BOUNCE_BOTH )
memcpy(b->ubuf, b->hbuf, b->sz);
+ fprintf(stderr, "%s bounced %zd bytes back from hcall buf %p into user buf %p\n", __func__, b->sz, b->hbuf, b->ubuf);
xc__hypercall_buffer_free(xch, b);
}
diff -r 86640f3de07f tools/libxc/xc_pm.c
--- a/tools/libxc/xc_pm.c Mon Nov 01 10:03:55 2010 +0000
+++ b/tools/libxc/xc_pm.c Mon Nov 01 10:21:05 2010 +0000
@@ -128,20 +128,32 @@ int xc_pm_get_cxstat(xc_interface *xch,
DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, &cxpt->residencies, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
int max_cx, ret;
- if( !cxpt || !(cxpt->triggers) || !(cxpt->residencies) )
- return -EINVAL;
+ if( !cxpt || !(cxpt->triggers) || !(cxpt->residencies) ) {
+ fprintf(stderr, "%s failing at %d\n", __func__, __LINE__);
+ errno = EINVAL;
+ return -1;
+ }
if ( (ret = xc_pm_get_max_cx(xch, cpuid, &max_cx)) )
+ {
+ fprintf(stderr, "%s failing at %d\n", __func__, __LINE__);
goto unlock_0;
+ }
HYPERCALL_BOUNCE_SET_SIZE(triggers, max_cx * sizeof(uint64_t));
HYPERCALL_BOUNCE_SET_SIZE(residencies, max_cx * sizeof(uint64_t));
ret = -1;
if ( xc_hypercall_bounce_pre(xch, triggers) )
+ {
+ fprintf(stderr, "%s failing at %d\n", __func__, __LINE__);
goto unlock_0;
+ }
if ( xc_hypercall_bounce_pre(xch, residencies) )
+ {
+ fprintf(stderr, "%s failing at %d\n", __func__, __LINE__);
goto unlock_1;
+ }
sysctl.cmd = XEN_SYSCTL_get_pmstat;
sysctl.u.get_pmstat.type = PMSTAT_get_cxstat;
@@ -150,7 +162,10 @@ int xc_pm_get_cxstat(xc_interface *xch,
set_xen_guest_handle(sysctl.u.get_pmstat.u.getcx.residencies, residencies);
if ( (ret = xc_sysctl(xch, &sysctl)) )
+ {
+ fprintf(stderr, "%s failing at %d\n", __func__, __LINE__);
goto unlock_2;
+ }
cxpt->nr = sysctl.u.get_pmstat.u.getcx.nr;
cxpt->last = sysctl.u.get_pmstat.u.getcx.last;
@@ -166,6 +181,7 @@ unlock_1:
unlock_1:
xc_hypercall_bounce_post(xch, triggers);
unlock_0:
+ fprintf(stderr, "%s done returning %d\n", __func__, ret);
return ret;
}
diff -r 86640f3de07f tools/misc/xenpm.c
--- a/tools/misc/xenpm.c Mon Nov 01 10:03:55 2010 +0000
+++ b/tools/misc/xenpm.c Mon Nov 01 10:21:05 2010 +0000
@@ -115,12 +115,15 @@ static int get_cxstat_by_cpuid(xc_interf
if ( !cxstat )
return -EINVAL;
+ fprintf(stderr, "get_cxstat_by_cpuid: max_cx %d for cpuid %d\n", max_cx_num, cpuid);
+
cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
if ( !cxstat->triggers )
return -ENOMEM;
cxstat->residencies = malloc(max_cx_num * sizeof(uint64_t));
if ( !cxstat->residencies )
{
+ fprintf(stderr, "failed to allocate residencies, freeing triggers\n");
free(cxstat->triggers);
return -ENOMEM;
}
@@ -129,13 +132,14 @@ static int get_cxstat_by_cpuid(xc_interf
if( ret )
{
int temp = errno;
+ fprintf(stderr, "xc_pm_get_cx_stat failed %d %d, freeing buffers\n", ret, errno);
free(cxstat->triggers);
free(cxstat->residencies);
cxstat->triggers = NULL;
cxstat->residencies = NULL;
return temp;
}
-
+ fprintf(stderr, "get_cx_stat_by_cpuid succeeded for cpu %d\n", cpuid);
return 0;
}
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: xenpm fail
2010-11-01 10:26 ` Ian Campbell
@ 2010-11-01 10:49 ` Zhang, Yang Z
2010-11-01 11:23 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Zhang, Yang Z @ 2010-11-01 10:49 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian, xen-devel, Jackson
[-- Attachment #1: Type: text/plain, Size: 5467 bytes --]
Attachment is our test source. Use "gcc -lxenctrl residency.c -o residency" to compile, then run "./ residency -n 1 -c" to get the c status data.
And following is the backtrace and output with your patch:
[root@vt-nhm7 tools]# gdb xenpm
GNU gdb (GDB) Red Hat Enterprise Linux (7.0.1-23.el5)
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /usr/sbin/xenpm...done.
(gdb) set args get-cpuidle-states
(gdb) r
Starting program: /usr/sbin/xenpm get-cpuidle-states
[Thread debugging using libthread_db enabled]
xc__hypercall_bounce_pre bounced 136 bytes from user buf 0x7fffffffe700 into hcall buf 0x607004
xc__hypercall_bounce_post bounced 136 bytes back from hcall buf 0x607004 into user buf 0x7fffffffe700
xc__hypercall_bounce_pre bounced 136 bytes from user buf 0x7fffffffe6d0 into hcall buf 0x607004
xc__hypercall_bounce_post bounced 136 bytes back from hcall buf 0x607004 into user buf 0x7fffffffe6d0
Max C-state: C7
xc__hypercall_bounce_pre bounced 136 bytes from user buf 0x7fffffffe620 into hcall buf 0x607004
xc__hypercall_bounce_post bounced 136 bytes back from hcall buf 0x607004 into user buf 0x7fffffffe620
get_cxstat_by_cpuid: max_cx 4 for cpuid 0
xc__hypercall_bounce_pre bounced 136 bytes from user buf 0x7fffffffe4f0 into hcall buf 0x607004
xc__hypercall_bounce_post bounced 136 bytes back from hcall buf 0x607004 into user buf 0x7fffffffe4f0
xc__hypercall_bounce_pre bounced 32 bytes from user buf 0x7fffffffe720 into hcall buf 0x607004
xc__hypercall_bounce_pre bounced 32 bytes from user buf 0x7fffffffe728 into hcall buf 0x609004
xc__hypercall_bounce_pre bounced 136 bytes from user buf 0x7fffffffe610 into hcall buf 0x60b004
xc__hypercall_bounce_post bounced 136 bytes back from hcall buf 0x60b004 into user buf 0x7fffffffe610
xc__hypercall_bounce_post bounced 32 bytes back from hcall buf 0x609004 into user buf 0x7fffffffe728
xc__hypercall_bounce_post bounced 32 bytes back from hcall buf 0x607004 into user buf 0x7fffffffe720
xc_pm_get_cxstat done returning 0
get_cx_stat_by_cpuid succeeded for cpu 0
cpu id : 0
total C-states : 4
idle time(ms) : 32842665
Program received signal SIGSEGV, Segmentation fault.
0x000000000040255a in print_cxstat (xc_handle=<value optimized out>, cpuid=<value optimized out>)
at xenpm.c:90
90 printf("C%d : transition [%020"PRIu64"]\n",
(gdb) bt
#0 0x000000000040255a in print_cxstat (xc_handle=<value optimized out>, cpuid=<value optimized out>)
at xenpm.c:90
#1 show_cxstat_by_cpuid (xc_handle=<value optimized out>, cpuid=<value optimized out>) at xenpm.c:167
#2 0x0000000000403a8b in cxstat_func (argc=<value optimized out>, argv=<value optimized out>)
at xenpm.c:191
#3 0x0000000000401394 in main (argc=2, argv=0x7fffffffe998) at xenpm.c:1177
best regards
yang
> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@eu.citrix.com]
> Sent: Monday, November 01, 2010 6:27 PM
> To: Zhang, Yang Z
> Cc: xen-devel@lists.xensource.com; Ian Jackson
> Subject: RE: xenpm fail
>
> On Mon, 2010-11-01 at 02:14 +0000, Zhang, Yang Z wrote:
> > > -----Original Message-----
> > > From: Ian Campbell [mailto:Ian.Campbell@eu.citrix.com]
> > > Sent: Friday, October 29, 2010 5:26 PM
> > > To: Zhang, Yang Z
> > > Cc: xen-devel@lists.xensource.com; Ian Jackson
> > > Subject: Re: xenpm fail
> > >
> > > On Fri, 2010-10-29 at 09:32 +0100, Zhang, Yang Z wrote:
> > > > Hi ian
> > > > I find Cs 22292 cause xenpm broken. When run "xenpm start" or
> > > > "xenpm get-cpuidle-states" and other xenmpm command, it will get
> > > > segment fault.
> > > > After do some investigation, I find call xc_pm_get_cxstat() will
> > > > free the cxstat->tiggers, For example:
> > > > Here is some code form my test.c.
> > > >
> > > > struct xc_cx_stat cxstatinfo, *cxstat = &cxstatinfo;
> > > >
> > > > cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
> > > >
> > > > if ( !cxstat->triggers ) {
> > > > printf("get memory fail");
> > > > return NOMEM;
> > > > }
> > > > ret = xc_pm_get_cxstat(xc_handle, cpu, cxstat);
> > >
> > > what is ret at this point?
> > >
> > ret = 0
>
> Are you running the precise code you give above? xc_pm_get_cxstat will return
> failure if cxstat->residencies is not initialised. This didn't change in
> 22292:a1b39d2b9001. I suspect the error you are seeing with your test.c may
> be due to this and unrelated to the problem(s) with xenpm.
>
> My guess is that 22292:a1b39d2b9001 added a new potential failure case to
> xc_pm_get_cxstat (the bounce buffer allocation) which is causing an error to be
> returned which is incorrectly handled in xenpm but unfortunately I can't see it
> from staring at the code.
>
> Please can you try the attached, increasingly desperate, debugging patch and
> send the complete output of running both your test case and xenpm.
>
> Please could you also run xenpm under gdb and grab a backtrace from the
> location of the segfault.
>
> Ian.
[-- Attachment #2: residency.c --]
[-- Type: text/plain, Size: 13740 bytes --]
// Copyright (C) <2008>, Intel Corporation
// Author: jiajun.xu@intel.com
//
// This program is free software; you can redistribute it and/or
// modify it under the terms of the GNU General Public License version
// 2 as published by the Free Software Foundation.
//
// This program is distributed in the hope that it will be useful, but
// WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
// General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program; if not, write to the Free Software Foundation,
// Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
//
#include <time.h>
#include <stdlib.h>
#include <stdio.h>
#include <sys/mman.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
#include <signal.h>
#include <xenctrl.h>
#include <xen/xen.h>
#include <string.h>
#include <sys/select.h>
#include <getopt.h>
#include <stdio.h>
#include <xen/version.h>
#include <inttypes.h>
int changeset_check(int xc_handle, int *cs)
{
char *delim = " ";
char *tmp;
int i = 1;
int ret = 0;
xen_changeset_info_t changeset;
ret = xc_version(xc_handle, XENVER_changeset, &changeset);
if ( ret )
{
printf("get xen changeset num failed\n");
return ret;
}
tmp = strtok(changeset, delim);
i++;
while ( tmp )
{
tmp = strtok(NULL, delim);
if ( i == 7 )
break;
i++;
}
delim=":";
tmp = strtok(tmp, delim);
*cs = atoi(tmp);
return ret;
}
int print_cx_pminfo(int xc_handle, int cpu)
{
int i, ret = 0;
int max_cx_num;
struct xc_cx_stat cxstatinfo, *cxstat = &cxstatinfo;
int cs_num = 0;
ret = changeset_check(xc_handle, &cs_num);
if ( ret )
return ret;
printf("==cpu %d==\n", cpu);
/******* get max cx number ************/
ret = xc_pm_get_max_cx(xc_handle, cpu, &max_cx_num);
if(ret)
{
printf("get_max_cx error(%d)\n", ret);
return ret;
}
//printf("max cx num = %d\n", max_cx_num);
/******* get cx statistic *************/
cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
cxstat->residencies = malloc(max_cx_num * sizeof(uint64_t));
ret = xc_pm_get_cxstat(xc_handle, cpu, cxstat);
if(ret)
{
printf("get_max_cxstat error(%d)\n", ret);
return ret;
}
printf("nr = %d last = C%d\n",
cxstat->nr, cxstat->last);
printf("idle_time %-15ld\n",
cxstat->idle_time/1000000UL);
printf(" trigger residency\n");
if ( cs_num >= 18935 )
{
for(i=0; i<max_cx_num; i++)
printf("C%-15d %-15lu %-15lu\n",
i,
cxstat->triggers[i],
cxstat->residencies[i]/1000000UL);
}
else
{
for(i=0; i<max_cx_num; i++)
printf("C%-15d %-15lu %-15lu\n",
i,
cxstat->triggers[i],
(cxstat->residencies[i]*1000000UL/3579)/1000000UL);
}
free(cxstat->triggers);
free(cxstat->residencies);
/********* reset cx statistic *********/
//ret = xc_pm_reset_cxstat(xc_handle, 0);
//if(ret)
// return ret;
return ret;
}
static int get_cxstat_by_cpuid(int xc_fd, int cpuid, struct xc_cx_stat *cxstat)
{
int ret = 0;
int max_cx_num = 0;
ret = xc_pm_get_max_cx(xc_fd, cpuid, &max_cx_num);
if ( ret )
return errno;
if ( !cxstat )
return -EINVAL;
cxstat->triggers = malloc(max_cx_num * sizeof(uint64_t));
if ( !cxstat->triggers )
return -ENOMEM;
cxstat->residencies = malloc(max_cx_num * sizeof(uint64_t));
if ( !cxstat->residencies )
{
free(cxstat->triggers);
return -ENOMEM;
}
ret = xc_pm_get_cxstat(xc_fd, cpuid, cxstat);
if( ret )
{
int temp = errno;
free(cxstat->triggers);
free(cxstat->residencies);
cxstat->triggers = NULL;
cxstat->residencies = NULL;
return temp;
}
return 0;
}
static int get_pxstat_by_cpuid(int xc_fd, int cpuid, struct xc_px_stat *pxstat)
{
int ret = 0;
int max_px_num = 0;
ret = xc_pm_get_max_px(xc_fd, cpuid, &max_px_num);
if ( ret )
return errno;
if ( !pxstat)
return -EINVAL;
pxstat->trans_pt = malloc(max_px_num * max_px_num *
sizeof(uint64_t));
if ( !pxstat->trans_pt )
return -ENOMEM;
pxstat->pt = malloc(max_px_num * sizeof(struct xc_px_val));
if ( !pxstat->pt )
{
free(pxstat->trans_pt);
return -ENOMEM;
}
ret = xc_pm_get_pxstat(xc_fd, cpuid, pxstat);
if( ret )
{
int temp = errno;
free(pxstat->trans_pt);
free(pxstat->pt);
pxstat->trans_pt = NULL;
pxstat->pt = NULL;
return temp;
}
return 0;
}
int percentage_gather_func(int xc_handle, int cpu_num, unsigned int wait_tm, int c_flag, int p_flag)
{
int i, j;
uint64_t usec_start, usec_end, elapsed_tm;
struct timeval tv;
struct xc_cx_stat *cxstat, *cxstat_start, *cxstat_end;
struct xc_px_stat *pxstat, *pxstat_start, *pxstat_end;
uint64_t *sum, *sum_cx, *sum_px;
int ret = 0;
int cs_num = 0;
ret = changeset_check(xc_handle, &cs_num);
if ( ret )
return ret;
if ( gettimeofday(&tv, NULL) == -1 )
{
fprintf(stderr, "failed to get timeofday\n");
return ;
}
usec_start = tv.tv_sec * 1000000UL + tv.tv_usec;
sum = malloc(sizeof(uint64_t) * 2 * cpu_num);
if ( sum == NULL )
return ;
cxstat = malloc(sizeof(struct xc_cx_stat) * 2 * cpu_num);
if ( cxstat == NULL )
{
free(sum);
return ;
}
pxstat = malloc(sizeof(struct xc_px_stat) * 2 * cpu_num);
if ( pxstat == NULL )
{
free(sum);
free(cxstat);
return ;
}
memset(sum, 0, sizeof(uint64_t) * 2 * cpu_num);
memset(cxstat, 0, sizeof(struct xc_cx_stat) * 2 * cpu_num);
memset(pxstat, 0, sizeof(struct xc_px_stat) * 2 * cpu_num);
sum_cx = sum;
sum_px = sum + cpu_num;
cxstat_start = cxstat;
cxstat_end = cxstat + cpu_num;
pxstat_start = pxstat;
pxstat_end = pxstat + cpu_num;
if ( get_cxstat_by_cpuid(xc_handle, 0, NULL) == -ENODEV &&
c_flag == 1)
{
fprintf(stderr, "Xen cpu idle is disabled!\n");
return ;
}
if( get_pxstat_by_cpuid(xc_handle, 0, NULL) == -ENODEV &&
p_flag == 1)
{
fprintf(stderr, "Xen frequency is disabled!\n");
return ;
}
for ( i = 0; i < cpu_num; i++ )
{
if ( c_flag == 1 )
get_cxstat_by_cpuid(xc_handle, i, &cxstat_start[i]);
if ( p_flag == 1 )
get_pxstat_by_cpuid(xc_handle, i, &pxstat_start[i]);
}
sleep(wait_tm);
if ( gettimeofday(&tv, NULL) == -1 )
{
fprintf(stderr, "failed to get timeofday\n");
return ;
}
usec_end = tv.tv_sec * 1000000UL + tv.tv_usec;
for ( i = 0; i < cpu_num; i++ )
{
if ( c_flag == 1 )
get_cxstat_by_cpuid(xc_handle, i, &cxstat_end[i]);
if ( p_flag == 1 )
get_pxstat_by_cpuid(xc_handle, i, &pxstat_end[i]);
}
elapsed_tm = (usec_end - usec_start);
for ( i = 0; i < cpu_num; i++ )
{
uint64_t temp;
uint64_t trigger;
printf("CPU%d:\n\tresidency\tpercentage\tave_residency\n", i);
if ( c_flag == 1 )
{
for ( j = 0; j < cxstat_end[i].nr; j++ )
{
temp = cxstat_end[i].residencies[j] -
cxstat_start[i].residencies[j];
trigger = cxstat_end[i].triggers[j] -
cxstat_start[i].triggers[j];
if ( trigger == 0 )
trigger = 1;
if ( cs_num >= 18935 )
printf(" C%d\t%"PRIu64"ms\t%.2f%%\t%"PRIu64"us\n", j,
temp / 1000000UL, temp * 100UL / ((double)elapsed_tm * 1000UL),
(temp / 1000UL) / trigger );
else
printf(" C%d\t%"PRIu64"ms\t%.2f%%\t%"PRIu64"us\n", j,
(temp * 1000000UL / 3579) / 1000000UL,
((temp * 1000000000UL / 3579) / 1000000UL) / ((double)elapsed_tm / 1000UL),
((temp * 1000000UL / 3579) / 1000UL) / trigger );
}
}
if ( p_flag == 1 )
{
for ( j = 0; j < pxstat_end[i].total; j++ )
{
temp = pxstat_end[i].pt[j].residency -
pxstat_start[i].pt[j].residency;
trigger = pxstat_end[i].pt[j].count -
pxstat_start[i].pt[j].count;
if ( trigger == 0 )
trigger = 1;
printf(" P%d\t%"PRIu64"ms\t%.2f%%\t%"PRIu64"us\n", j,
temp / 1000000UL, temp * 100UL / ((double)elapsed_tm * 1000UL),
(temp / 1000UL) / trigger );
}
}
}
for ( i = 0; i < 2 * cpu_num; i++ )
{
free(cxstat[i].triggers);
free(cxstat[i].residencies);
free(pxstat[i].trans_pt);
free(pxstat[i].pt);
}
free(cxstat);
free(pxstat);
free(sum);
xc_interface_close(xc_handle);
exit(0);
}
int print_px_pminfo(int xc_handle, int cpu)
{
int max_px_num;
int i, ret = 0;
struct xc_px_stat pxstatinfo, *pxstat = &pxstatinfo;
printf("==cpu %d==\n", cpu);
ret = xc_pm_get_max_px(xc_handle, cpu, &max_px_num);
if(ret)
{
printf("get_max_px error(%d)\n", ret);
return ret;
}
printf("max px num = %d\n", max_px_num);
pxstat->trans_pt = malloc(max_px_num * max_px_num * sizeof(uint64_t));
pxstat->pt = malloc(max_px_num * sizeof(struct xc_px_val));
ret = xc_pm_get_pxstat(xc_handle, cpu, pxstat);
if(ret)
{
printf("get_max_pxstat error(%d)\n", ret);
return ret;
}
printf("total = %d, usable = %d, last = %d, cur = %d\n",
pxstat->total, pxstat->usable, pxstat->last, pxstat->cur);
printf(" freq residency count\n");
for(i=0; i<max_px_num; i++)
printf("P%-15d %-15lu %-15lu %-15lu\n",
i,
pxstat->pt[i].freq,
pxstat->pt[i].residency/1000000UL,
pxstat->pt[i].count);
/*for(i=0; i<max_px_num*max_px_num; i++) {
if(!(i%4))
printf("\n");
printf("%-10lu", pxstat->trans_pt[i]);
}
printf("\n");*/
free(pxstat->trans_pt);
free(pxstat->pt);
return ret;
}
int main(int argc, char **argv)
{
int c;
extern char *optarg;
extern int optind;
int cpu_num = 1;
int p_policy = 0;
int c_policy = 0;
int percentage = 0;
int cpu;
int ret = 0;
int reset = 0;
int wait_tm = 0;
xc_physinfo_t physinfo = { 0 };
while ((c = getopt(argc, argv, "rcpn:s:")) != -1) {
switch (c) {
case 'n':
cpu_num = atoi(optarg);
break;
case 'p':
p_policy = 1;
break;
case 'c':
c_policy = 1;
break;
case 'r':
reset = 1;
break;
case 's':
percentage = 1;
wait_tm = atoi(optarg);
break;
default:
printf("You should set CPU num\n");
exit(1);
}
}
xc_interface *xc_handle = xc_interface_open(0,0,0);
if (!xc_handle)
{
fprintf(stderr, "failed to get the handler\n");
return 0;
}
ret = xc_physinfo(xc_handle, &physinfo);
if ( ret )
{
fprintf(stderr, "failed to get the processor information\n");
xc_interface_close(xc_handle);
return 0;
}
if ( cpu_num == 1 )
cpu_num = physinfo.nr_cpus;
if ( reset == 1 )
{
printf("Reset statistic data\n");
for ( cpu = 0; cpu < cpu_num; cpu++ )
{
if ( p_policy == 1 )
{
ret = xc_pm_reset_pxstat(xc_handle, cpu);
if (ret)
{
printf("reset px statistic info error(%d)\n", ret);
exit(ret);
}
}
if ( c_policy == 1 )
{
ret = xc_pm_reset_cxstat(xc_handle, cpu);
if (ret)
{
printf("reset cx statistic info error(%d)\n", ret);
exit(ret);
}
}
}
return xc_interface_close(xc_handle);
}
if ( percentage == 1 )
{
return percentage_gather_func(xc_handle, cpu_num, wait_tm, c_policy, p_policy);
}
printf("=====begin to collect statistic collection=====\n");
for ( cpu = 0; cpu < cpu_num; cpu++ )
{
if ( p_policy == 1 )
{
ret = print_px_pminfo(xc_handle, cpu);
if (ret)
{
printf("get px statistic info error(%d)\n", ret);
exit(ret);
}
}
if ( c_policy == 1 )
ret = print_cx_pminfo(xc_handle, cpu);
if (ret)
{
printf("get cx statistic info error(%d)\n", ret);
exit(ret);
}
}
printf("=====end of statistic data collecton=====\n");
return xc_interface_close(xc_handle);
}
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: xenpm fail
2010-11-01 10:49 ` Zhang, Yang Z
@ 2010-11-01 11:23 ` Ian Campbell
2010-11-01 12:26 ` Zhang, Yang Z
0 siblings, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2010-11-01 11:23 UTC (permalink / raw)
To: Zhang, Yang Z; +Cc: xen-devel, Ian Jackson
Thanks for this...
On Mon, 2010-11-01 at 10:49 +0000, Zhang, Yang Z wrote:
[...]
> xc__hypercall_bounce_pre bounced 32 bytes from user buf 0x7fffffffe720 into hcall buf 0x607004
> xc__hypercall_bounce_pre bounced 32 bytes from user buf 0x7fffffffe728 into hcall buf 0x609004
> xc__hypercall_bounce_pre bounced 136 bytes from user buf 0x7fffffffe610 into hcall buf 0x60b004
> xc__hypercall_bounce_post bounced 136 bytes back from hcall buf 0x60b004 into user buf 0x7fffffffe610
> xc__hypercall_bounce_post bounced 32 bytes back from hcall buf 0x609004 into user buf 0x7fffffffe728
> xc__hypercall_bounce_post bounced 32 bytes back from hcall buf 0x607004 into user buf 0x7fffffffe720
This is the xc_pm_get_cxstat call, we can see it bounce max_cx(=4) *
sizeof(uint64_t)==32 bytes for both cxpt->triggers and cxpt->residencies
as well as 136 bytes for struct xensysctl.
However the ubuf values for triggers and residencies are suspicious,
they are only 8 bytes different, IOW they apparently overlap.
Can you try this patch which fixes a stupid thinko.
diff -r c3d7d2729410 tools/libxc/xc_pm.c
--- a/tools/libxc/xc_pm.c Mon Nov 01 11:12:51 2010 +0000
+++ b/tools/libxc/xc_pm.c Mon Nov 01 11:19:53 2010 +0000
@@ -124,8 +124,8 @@ int xc_pm_get_cxstat(xc_interface *xch,
int xc_pm_get_cxstat(xc_interface *xch, int cpuid, struct xc_cx_stat *cxpt)
{
DECLARE_SYSCTL;
- DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, &cxpt->triggers, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
- DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, &cxpt->residencies, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, cxpt->triggers, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, cxpt->residencies, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
int max_cx, ret;
if( !cxpt || !(cxpt->triggers) || !(cxpt->residencies) )
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: xenpm fail
2010-11-01 11:23 ` Ian Campbell
@ 2010-11-01 12:26 ` Zhang, Yang Z
2010-11-01 13:14 ` Ian Campbell
0 siblings, 1 reply; 11+ messages in thread
From: Zhang, Yang Z @ 2010-11-01 12:26 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian, xen-devel, Jackson
[-- Attachment #1: Type: text/plain, Size: 3891 bytes --]
"Xenpm get-cpuidle-states" works well with your patch, but "xenpm start" still get segment fault. And with following patch, I didn't see any segment fault when run xenpm or my test case. Maybe this problem will solved by this patch. Pls take a look at it.
diff -r a1b39d2b9001 tools/libxc/xc_pm.c
--- a/tools/libxc/xc_pm.c Fri Oct 22 15:14:51 2010 +0100
+++ b/tools/libxc/xc_pm.c Tue Nov 02 04:06:10 2010 +0800
@@ -46,8 +46,8 @@ int xc_pm_get_pxstat(xc_interface *xch,
{
DECLARE_SYSCTL;
/* Sizes unknown until xc_pm_get_max_px */
- DECLARE_NAMED_HYPERCALL_BOUNCE(trans, &pxpt->trans_pt, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
- DECLARE_NAMED_HYPERCALL_BOUNCE(pt, &pxpt->pt, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ DECLARE_NAMED_HYPERCALL_BOUNCE(trans, pxpt->trans_pt, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ DECLARE_NAMED_HYPERCALL_BOUNCE(pt, pxpt->pt, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
int max_px, ret;
@@ -124,8 +124,8 @@ int xc_pm_get_cxstat(xc_interface *xch,
int xc_pm_get_cxstat(xc_interface *xch, int cpuid, struct xc_cx_stat *cxpt)
{
DECLARE_SYSCTL;
- DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, &cxpt->triggers, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
- DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, &cxpt->residencies, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, cxpt->triggers, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+ DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, cxpt->residencies, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
int max_cx, ret;
if( !cxpt || !(cxpt->triggers) || !(cxpt->residencies) )
best regards
yang
> -----Original Message-----
> From: Ian Campbell [mailto:Ian.Campbell@eu.citrix.com]
> Sent: Monday, November 01, 2010 7:24 PM
> To: Zhang, Yang Z
> Cc: xen-devel@lists.xensource.com; Ian Jackson
> Subject: RE: xenpm fail
>
> Thanks for this...
>
> On Mon, 2010-11-01 at 10:49 +0000, Zhang, Yang Z wrote:
> [...]
> > xc__hypercall_bounce_pre bounced 32 bytes from user buf 0x7fffffffe720 into
> hcall buf 0x607004
> > xc__hypercall_bounce_pre bounced 32 bytes from user buf 0x7fffffffe728 into
> hcall buf 0x609004
> > xc__hypercall_bounce_pre bounced 136 bytes from user buf 0x7fffffffe610
> into hcall buf 0x60b004
> > xc__hypercall_bounce_post bounced 136 bytes back from hcall buf 0x60b004
> into user buf 0x7fffffffe610
> > xc__hypercall_bounce_post bounced 32 bytes back from hcall buf 0x609004
> into user buf 0x7fffffffe728
> > xc__hypercall_bounce_post bounced 32 bytes back from hcall buf 0x607004
> into user buf 0x7fffffffe720
>
> This is the xc_pm_get_cxstat call, we can see it bounce max_cx(=4) *
> sizeof(uint64_t)==32 bytes for both cxpt->triggers and cxpt->residencies
> as well as 136 bytes for struct xensysctl.
>
> However the ubuf values for triggers and residencies are suspicious,
> they are only 8 bytes different, IOW they apparently overlap.
>
> Can you try this patch which fixes a stupid thinko.
>
> diff -r c3d7d2729410 tools/libxc/xc_pm.c
> --- a/tools/libxc/xc_pm.c Mon Nov 01 11:12:51 2010 +0000
> +++ b/tools/libxc/xc_pm.c Mon Nov 01 11:19:53 2010 +0000
> @@ -124,8 +124,8 @@ int xc_pm_get_cxstat(xc_interface *xch,
> int xc_pm_get_cxstat(xc_interface *xch, int cpuid, struct xc_cx_stat *cxpt)
> {
> DECLARE_SYSCTL;
> - DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, &cxpt->triggers, 0,
> XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> - DECLARE_NAMED_HYPERCALL_BOUNCE(residencies,
> &cxpt->residencies, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> + DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, cxpt->triggers, 0,
> XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> + DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, cxpt->residencies,
> 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> int max_cx, ret;
>
> if( !cxpt || !(cxpt->triggers) || !(cxpt->residencies) )
>
[-- Attachment #2: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: xenpm fail
2010-11-01 12:26 ` Zhang, Yang Z
@ 2010-11-01 13:14 ` Ian Campbell
0 siblings, 0 replies; 11+ messages in thread
From: Ian Campbell @ 2010-11-01 13:14 UTC (permalink / raw)
To: Zhang, Yang Z; +Cc: xen-devel, Ian Jackson
On Mon, 2010-11-01 at 12:26 +0000, Zhang, Yang Z wrote:
> "Xenpm get-cpuidle-states" works well with your patch, but "xenpm
> start" still get segment fault. And with following patch, I didn't see
> any segment fault when run xenpm or my test case. Maybe this problem
> will solved by this patch. Pls take a look at it.
Thanks, I came up with the same by looking for all the & used with these
macros.
Will submit a patch shortly.
Ian.
>
> diff -r a1b39d2b9001 tools/libxc/xc_pm.c
> --- a/tools/libxc/xc_pm.c Fri Oct 22 15:14:51 2010 +0100
> +++ b/tools/libxc/xc_pm.c Tue Nov 02 04:06:10 2010 +0800
> @@ -46,8 +46,8 @@ int xc_pm_get_pxstat(xc_interface *xch,
> {
> DECLARE_SYSCTL;
> /* Sizes unknown until xc_pm_get_max_px */
> - DECLARE_NAMED_HYPERCALL_BOUNCE(trans, &pxpt->trans_pt, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> - DECLARE_NAMED_HYPERCALL_BOUNCE(pt, &pxpt->pt, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> + DECLARE_NAMED_HYPERCALL_BOUNCE(trans, pxpt->trans_pt, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> + DECLARE_NAMED_HYPERCALL_BOUNCE(pt, pxpt->pt, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
>
> int max_px, ret;
>
> @@ -124,8 +124,8 @@ int xc_pm_get_cxstat(xc_interface *xch,
> int xc_pm_get_cxstat(xc_interface *xch, int cpuid, struct xc_cx_stat *cxpt)
> {
> DECLARE_SYSCTL;
> - DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, &cxpt->triggers, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> - DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, &cxpt->residencies, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> + DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, cxpt->triggers, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> + DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, cxpt->residencies, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> int max_cx, ret;
>
> if( !cxpt || !(cxpt->triggers) || !(cxpt->residencies) )
>
> best regards
> yang
>
> > -----Original Message-----
> > From: Ian Campbell [mailto:Ian.Campbell@eu.citrix.com]
> > Sent: Monday, November 01, 2010 7:24 PM
> > To: Zhang, Yang Z
> > Cc: xen-devel@lists.xensource.com; Ian Jackson
> > Subject: RE: xenpm fail
> >
> > Thanks for this...
> >
> > On Mon, 2010-11-01 at 10:49 +0000, Zhang, Yang Z wrote:
> > [...]
> > > xc__hypercall_bounce_pre bounced 32 bytes from user buf 0x7fffffffe720 into
> > hcall buf 0x607004
> > > xc__hypercall_bounce_pre bounced 32 bytes from user buf 0x7fffffffe728 into
> > hcall buf 0x609004
> > > xc__hypercall_bounce_pre bounced 136 bytes from user buf 0x7fffffffe610
> > into hcall buf 0x60b004
> > > xc__hypercall_bounce_post bounced 136 bytes back from hcall buf 0x60b004
> > into user buf 0x7fffffffe610
> > > xc__hypercall_bounce_post bounced 32 bytes back from hcall buf 0x609004
> > into user buf 0x7fffffffe728
> > > xc__hypercall_bounce_post bounced 32 bytes back from hcall buf 0x607004
> > into user buf 0x7fffffffe720
> >
> > This is the xc_pm_get_cxstat call, we can see it bounce max_cx(=4) *
> > sizeof(uint64_t)==32 bytes for both cxpt->triggers and cxpt->residencies
> > as well as 136 bytes for struct xensysctl.
> >
> > However the ubuf values for triggers and residencies are suspicious,
> > they are only 8 bytes different, IOW they apparently overlap.
> >
> > Can you try this patch which fixes a stupid thinko.
> >
> > diff -r c3d7d2729410 tools/libxc/xc_pm.c
> > --- a/tools/libxc/xc_pm.c Mon Nov 01 11:12:51 2010 +0000
> > +++ b/tools/libxc/xc_pm.c Mon Nov 01 11:19:53 2010 +0000
> > @@ -124,8 +124,8 @@ int xc_pm_get_cxstat(xc_interface *xch,
> > int xc_pm_get_cxstat(xc_interface *xch, int cpuid, struct xc_cx_stat *cxpt)
> > {
> > DECLARE_SYSCTL;
> > - DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, &cxpt->triggers, 0,
> > XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> > - DECLARE_NAMED_HYPERCALL_BOUNCE(residencies,
> > &cxpt->residencies, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> > + DECLARE_NAMED_HYPERCALL_BOUNCE(triggers, cxpt->triggers, 0,
> > XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> > + DECLARE_NAMED_HYPERCALL_BOUNCE(residencies, cxpt->residencies,
> > 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
> > int max_cx, ret;
> >
> > if( !cxpt || !(cxpt->triggers) || !(cxpt->residencies) )
> >
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-11-01 13:14 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-29 8:32 xenpm fail Zhang, Yang Z
2010-10-29 8:55 ` Ian Campbell
2010-10-29 9:26 ` Ian Campbell
2010-10-29 9:42 ` Ian Campbell
2010-11-01 2:17 ` Zhang, Yang Z
2010-11-01 2:14 ` Zhang, Yang Z
2010-11-01 10:26 ` Ian Campbell
2010-11-01 10:49 ` Zhang, Yang Z
2010-11-01 11:23 ` Ian Campbell
2010-11-01 12:26 ` Zhang, Yang Z
2010-11-01 13:14 ` Ian Campbell
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.