* [PATCH] tools/misc/xenpm: fix getting info when some CPUs are offline
@ 2018-10-30 22:16 Marek Marczykowski-Górecki
2018-10-30 22:29 ` Andrew Cooper
2018-10-31 9:33 ` Jan Beulich
0 siblings, 2 replies; 5+ messages in thread
From: Marek Marczykowski-Górecki @ 2018-10-30 22:16 UTC (permalink / raw)
To: xen-devel; +Cc: Wei Liu, Ian Jackson, Marek Marczykowski-Górecki
Use physinfo.max_cpu_id instead of physinfo.nr_cpus to get max CPU id.
This fixes for example 'xenpm get-cpufreq-para' with smt=off, which
otherwise would miss half of the cores.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Reported by @tfm1:
https://github.com/QubesOS/qubes-issues/isues/4456
---
tools/misc/xenpm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index 86c12ea5fb..112590fb67 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -1231,7 +1231,7 @@ int main(int argc, char *argv[])
xc_interface_close(xc_handle);
return ret;
}
- max_cpu_nr = physinfo.nr_cpus;
+ max_cpu_nr = physinfo.max_cpu_id;
/* calculate how many options match with user's input */
for ( i = 0; i < ARRAY_SIZE(main_options); i++ )
--
2.17.2
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/misc/xenpm: fix getting info when some CPUs are offline
2018-10-30 22:16 [PATCH] tools/misc/xenpm: fix getting info when some CPUs are offline Marek Marczykowski-Górecki
@ 2018-10-30 22:29 ` Andrew Cooper
2018-10-31 9:33 ` Jan Beulich
1 sibling, 0 replies; 5+ messages in thread
From: Andrew Cooper @ 2018-10-30 22:29 UTC (permalink / raw)
To: Marek Marczykowski-Górecki, xen-devel; +Cc: Ian Jackson, Wei Liu
On 30/10/2018 22:16, Marek Marczykowski-Górecki wrote:
> Use physinfo.max_cpu_id instead of physinfo.nr_cpus to get max CPU id.
> This fixes for example 'xenpm get-cpufreq-para' with smt=off, which
> otherwise would miss half of the cores.
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
> Reported by @tfm1:
> https://github.com/QubesOS/qubes-issues/isues/4456
Thats a 404, but /issues/ looks to work.
Either way, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> tools/misc/xenpm.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
> index 86c12ea5fb..112590fb67 100644
> --- a/tools/misc/xenpm.c
> +++ b/tools/misc/xenpm.c
> @@ -1231,7 +1231,7 @@ int main(int argc, char *argv[])
> xc_interface_close(xc_handle);
> return ret;
> }
> - max_cpu_nr = physinfo.nr_cpus;
> + max_cpu_nr = physinfo.max_cpu_id;
>
> /* calculate how many options match with user's input */
> for ( i = 0; i < ARRAY_SIZE(main_options); i++ )
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/misc/xenpm: fix getting info when some CPUs are offline
2018-10-30 22:16 [PATCH] tools/misc/xenpm: fix getting info when some CPUs are offline Marek Marczykowski-Górecki
2018-10-30 22:29 ` Andrew Cooper
@ 2018-10-31 9:33 ` Jan Beulich
2018-10-31 10:53 ` Marek Marczykowski
1 sibling, 1 reply; 5+ messages in thread
From: Jan Beulich @ 2018-10-31 9:33 UTC (permalink / raw)
To: Marek Marczykowski; +Cc: Ian Jackson, Wei Liu, xen-devel
>>> On 30.10.18 at 23:16, <marmarek@invisiblethingslab.com> wrote:
> --- a/tools/misc/xenpm.c
> +++ b/tools/misc/xenpm.c
> @@ -1231,7 +1231,7 @@ int main(int argc, char *argv[])
> xc_interface_close(xc_handle);
> return ret;
> }
> - max_cpu_nr = physinfo.nr_cpus;
> + max_cpu_nr = physinfo.max_cpu_id;
Isn't this off by 1 then? max_cpu_nr is misnamed, all loops using it
are of the form
for ( i = 0; i < max_cpu_nr; i++ )
I'm also afraid there are further quirks here, with various constructs
along the lines of (as bodies of aforementioned for())
if ( show_cxstat_by_cpuid(xc_handle, i) == -ENODEV )
break;
which I suspect would terminate processing early when hitting a true
gap (i.e. not one resulting from a parked CPU). But I guess it wouldn't
be appropriate to ask you to deal with this at the same time.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/misc/xenpm: fix getting info when some CPUs are offline
2018-10-31 9:33 ` Jan Beulich
@ 2018-10-31 10:53 ` Marek Marczykowski
2018-10-31 11:11 ` Jan Beulich
0 siblings, 1 reply; 5+ messages in thread
From: Marek Marczykowski @ 2018-10-31 10:53 UTC (permalink / raw)
To: Jan Beulich; +Cc: Ian Jackson, Wei Liu, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1488 bytes --]
On Wed, Oct 31, 2018 at 03:33:58AM -0600, Jan Beulich wrote:
> >>> On 30.10.18 at 23:16, <marmarek@invisiblethingslab.com> wrote:
> > --- a/tools/misc/xenpm.c
> > +++ b/tools/misc/xenpm.c
> > @@ -1231,7 +1231,7 @@ int main(int argc, char *argv[])
> > xc_interface_close(xc_handle);
> > return ret;
> > }
> > - max_cpu_nr = physinfo.nr_cpus;
> > + max_cpu_nr = physinfo.max_cpu_id;
>
> Isn't this off by 1 then? max_cpu_nr is misnamed, all loops using it
> are of the form
>
> for ( i = 0; i < max_cpu_nr; i++ )
Oh, you're right. This doesn't affect smt=off case, because the last cpu
is offline anyway...
> I'm also afraid there are further quirks here, with various constructs
> along the lines of (as bodies of aforementioned for())
>
> if ( show_cxstat_by_cpuid(xc_handle, i) == -ENODEV )
> break;
>
> which I suspect would terminate processing early when hitting a true
> gap (i.e. not one resulting from a parked CPU).
How that would happen? Hotplug?
Also I've tried to look at error codes to distinguish offline cpu and
avoid printing that error, but looks EINVAL is used for a lot of cases.
> But I guess it wouldn't
> be appropriate to ask you to deal with this at the same time.
>
> Jan
>
>
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 157 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tools/misc/xenpm: fix getting info when some CPUs are offline
2018-10-31 10:53 ` Marek Marczykowski
@ 2018-10-31 11:11 ` Jan Beulich
0 siblings, 0 replies; 5+ messages in thread
From: Jan Beulich @ 2018-10-31 11:11 UTC (permalink / raw)
To: Marek Marczykowski; +Cc: Ian Jackson, Wei Liu, xen-devel
>>> On 31.10.18 at 11:53, <marmarek@invisiblethingslab.com> wrote:
> On Wed, Oct 31, 2018 at 03:33:58AM -0600, Jan Beulich wrote:
>> I'm also afraid there are further quirks here, with various constructs
>> along the lines of (as bodies of aforementioned for())
>>
>> if ( show_cxstat_by_cpuid(xc_handle, i) == -ENODEV )
>> break;
>>
>> which I suspect would terminate processing early when hitting a true
>> gap (i.e. not one resulting from a parked CPU).
>
> How that would happen? Hotplug?
xen-hptool on AMD might trigger this, afaict.
> Also I've tried to look at error codes to distinguish offline cpu and
> avoid printing that error, but looks EINVAL is used for a lot of cases.
Yeah, the common problem with EINVAL.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-31 11:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-30 22:16 [PATCH] tools/misc/xenpm: fix getting info when some CPUs are offline Marek Marczykowski-Górecki
2018-10-30 22:29 ` Andrew Cooper
2018-10-31 9:33 ` Jan Beulich
2018-10-31 10:53 ` Marek Marczykowski
2018-10-31 11:11 ` Jan Beulich
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.