* [PATCH] sysctl: adjust XEN_SYSCTL_cputopoinfo behavior
@ 2015-07-13 15:53 Jan Beulich
2015-07-13 17:43 ` Boris Ostrovsky
2015-07-15 15:51 ` Ian Campbell
0 siblings, 2 replies; 4+ messages in thread
From: Jan Beulich @ 2015-07-13 15:53 UTC (permalink / raw)
To: xen-devel; +Cc: Ian Campbell, Keir Fraser, Ian Jackson, Tim Deegan
[-- Attachment #1: Type: text/plain, Size: 2169 bytes --]
The new function's implementation, other than the original one of
XEN_SYSCTL_topologyinfo, didn't allow the caller to get what it needs
(if e.g. it's after the data for just one specific CPU) with just one
hypercall, without caring about the total number of CPUs in the system.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -358,15 +358,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
{
xen_sysctl_cputopo_t cputopo = { 0 };
- if ( ti->num_cpus < num_cpus )
- {
- ret = -ENOBUFS;
- i = num_cpus;
- }
- else
- i = 0;
-
- for ( ; i < num_cpus; i++ )
+ if ( num_cpus > ti->num_cpus )
+ num_cpus = ti->num_cpus;
+ for ( i = 0; i < num_cpus; ++i )
{
if ( cpu_present(i) )
{
@@ -393,7 +387,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
else
i = num_cpus;
- if ( (!ret || (ret == -ENOBUFS)) && (ti->num_cpus != i) )
+ if ( !ret && (ti->num_cpus != i) )
{
ti->num_cpus = i;
if ( __copy_field_to_guest(u_sysctl, op,
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -482,10 +482,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cputo
* - otherwise it's the number of entries in 'cputopo'
*
* OUT:
- * - If 'num_cpus' is less than the number Xen needs to write, -ENOBUFS shall
- * be returned and 'num_cpus' updated to reflect the intended number.
- * - On success, 'num_cpus' shall indicate the number of entries written, which
- * may be less than the maximum.
+ * - If 'num_cpus' is less than the number Xen wants to write but the handle
+ * handle is not a NULL one, partial data gets returned and 'num_cpus' gets
+ * updated to reflect the intended number.
+ * - Otherwise, 'num_cpus' shall indicate the number of entries written, which
+ * may be less than the input value.
*/
struct xen_sysctl_cputopoinfo {
uint32_t num_cpus;
[-- Attachment #2: sysctl-cputopoinfo-partial.patch --]
[-- Type: text/plain, Size: 2213 bytes --]
sysctl: adjust XEN_SYSCTL_cputopoinfo behavior
The new function's implementation, other than the original one of
XEN_SYSCTL_topologyinfo, didn't allow the caller to get what it needs
(if e.g. it's after the data for just one specific CPU) with just one
hypercall, without caring about the total number of CPUs in the system.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -358,15 +358,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
{
xen_sysctl_cputopo_t cputopo = { 0 };
- if ( ti->num_cpus < num_cpus )
- {
- ret = -ENOBUFS;
- i = num_cpus;
- }
- else
- i = 0;
-
- for ( ; i < num_cpus; i++ )
+ if ( num_cpus > ti->num_cpus )
+ num_cpus = ti->num_cpus;
+ for ( i = 0; i < num_cpus; ++i )
{
if ( cpu_present(i) )
{
@@ -393,7 +387,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
else
i = num_cpus;
- if ( (!ret || (ret == -ENOBUFS)) && (ti->num_cpus != i) )
+ if ( !ret && (ti->num_cpus != i) )
{
ti->num_cpus = i;
if ( __copy_field_to_guest(u_sysctl, op,
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -482,10 +482,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cputo
* - otherwise it's the number of entries in 'cputopo'
*
* OUT:
- * - If 'num_cpus' is less than the number Xen needs to write, -ENOBUFS shall
- * be returned and 'num_cpus' updated to reflect the intended number.
- * - On success, 'num_cpus' shall indicate the number of entries written, which
- * may be less than the maximum.
+ * - If 'num_cpus' is less than the number Xen wants to write but the handle
+ * handle is not a NULL one, partial data gets returned and 'num_cpus' gets
+ * updated to reflect the intended number.
+ * - Otherwise, 'num_cpus' shall indicate the number of entries written, which
+ * may be less than the input value.
*/
struct xen_sysctl_cputopoinfo {
uint32_t num_cpus;
[-- 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] 4+ messages in thread
* Re: [PATCH] sysctl: adjust XEN_SYSCTL_cputopoinfo behavior
2015-07-13 15:53 [PATCH] sysctl: adjust XEN_SYSCTL_cputopoinfo behavior Jan Beulich
@ 2015-07-13 17:43 ` Boris Ostrovsky
2015-07-14 9:41 ` Jan Beulich
2015-07-15 15:51 ` Ian Campbell
1 sibling, 1 reply; 4+ messages in thread
From: Boris Ostrovsky @ 2015-07-13 17:43 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Ian Campbell, Ian Jackson, Keir Fraser, Tim Deegan
On 07/13/2015 11:53 AM, Jan Beulich wrote:
> The new function's implementation, other than the original one of
> XEN_SYSCTL_topologyinfo, didn't allow the caller to get what it needs
> (if e.g. it's after the data for just one specific CPU) with just one
> hypercall, without caring about the total number of CPUs in the system.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Would we want a similar update for XEN_SYSCTL_numainfo?
-boris
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -358,15 +358,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
> {
> xen_sysctl_cputopo_t cputopo = { 0 };
>
> - if ( ti->num_cpus < num_cpus )
> - {
> - ret = -ENOBUFS;
> - i = num_cpus;
> - }
> - else
> - i = 0;
> -
> - for ( ; i < num_cpus; i++ )
> + if ( num_cpus > ti->num_cpus )
> + num_cpus = ti->num_cpus;
> + for ( i = 0; i < num_cpus; ++i )
> {
> if ( cpu_present(i) )
> {
> @@ -393,7 +387,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
> else
> i = num_cpus;
>
> - if ( (!ret || (ret == -ENOBUFS)) && (ti->num_cpus != i) )
> + if ( !ret && (ti->num_cpus != i) )
> {
> ti->num_cpus = i;
> if ( __copy_field_to_guest(u_sysctl, op,
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -482,10 +482,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cputo
> * - otherwise it's the number of entries in 'cputopo'
> *
> * OUT:
> - * - If 'num_cpus' is less than the number Xen needs to write, -ENOBUFS shall
> - * be returned and 'num_cpus' updated to reflect the intended number.
> - * - On success, 'num_cpus' shall indicate the number of entries written, which
> - * may be less than the maximum.
> + * - If 'num_cpus' is less than the number Xen wants to write but the handle
> + * handle is not a NULL one, partial data gets returned and 'num_cpus' gets
> + * updated to reflect the intended number.
> + * - Otherwise, 'num_cpus' shall indicate the number of entries written, which
> + * may be less than the input value.
> */
> struct xen_sysctl_cputopoinfo {
> uint32_t num_cpus;
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sysctl: adjust XEN_SYSCTL_cputopoinfo behavior
2015-07-13 17:43 ` Boris Ostrovsky
@ 2015-07-14 9:41 ` Jan Beulich
0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2015-07-14 9:41 UTC (permalink / raw)
To: Boris Ostrovsky
Cc: Ian Campbell, xen-devel, Keir Fraser, Ian Jackson, Tim Deegan
>>> On 13.07.15 at 19:43, <boris.ostrovsky@oracle.com> wrote:
> On 07/13/2015 11:53 AM, Jan Beulich wrote:
>> The new function's implementation, other than the original one of
>> XEN_SYSCTL_topologyinfo, didn't allow the caller to get what it needs
>> (if e.g. it's after the data for just one specific CPU) with just one
>> hypercall, without caring about the total number of CPUs in the system.
>
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>
> Would we want a similar update for XEN_SYSCTL_numainfo?
I thought there was a second such case, but seeing
XEN_SYSCTL_pcitopoinfo not be it I didn't look further assuming
I mis-remembered. So yes, imo that one should be adjusted in the
same way.
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sysctl: adjust XEN_SYSCTL_cputopoinfo behavior
2015-07-13 15:53 [PATCH] sysctl: adjust XEN_SYSCTL_cputopoinfo behavior Jan Beulich
2015-07-13 17:43 ` Boris Ostrovsky
@ 2015-07-15 15:51 ` Ian Campbell
1 sibling, 0 replies; 4+ messages in thread
From: Ian Campbell @ 2015-07-15 15:51 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Ian Jackson, Tim Deegan
On Mon, 2015-07-13 at 16:53 +0100, Jan Beulich wrote:
> The new function's implementation, other than the original one of
FWIW "other than the" would normally be expressed as "unlike the".
> XEN_SYSCTL_topologyinfo, didn't allow the caller to get what it needs
> (if e.g. it's after the data for just one specific CPU) with just one
> hypercall, without caring about the total number of CPUs in the system.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -358,15 +358,9 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
> {
> xen_sysctl_cputopo_t cputopo = { 0 };
>
> - if ( ti->num_cpus < num_cpus )
> - {
> - ret = -ENOBUFS;
> - i = num_cpus;
> - }
> - else
> - i = 0;
> -
> - for ( ; i < num_cpus; i++ )
> + if ( num_cpus > ti->num_cpus )
> + num_cpus = ti->num_cpus;
> + for ( i = 0; i < num_cpus; ++i )
> {
> if ( cpu_present(i) )
> {
> @@ -393,7 +387,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xe
> else
> i = num_cpus;
>
> - if ( (!ret || (ret == -ENOBUFS)) && (ti->num_cpus != i) )
> + if ( !ret && (ti->num_cpus != i) )
> {
> ti->num_cpus = i;
> if ( __copy_field_to_guest(u_sysctl, op,
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -482,10 +482,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cputo
> * - otherwise it's the number of entries in 'cputopo'
> *
> * OUT:
> - * - If 'num_cpus' is less than the number Xen needs to write, -ENOBUFS shall
> - * be returned and 'num_cpus' updated to reflect the intended number.
> - * - On success, 'num_cpus' shall indicate the number of entries written, which
> - * may be less than the maximum.
> + * - If 'num_cpus' is less than the number Xen wants to write but the handle
> + * handle is not a NULL one, partial data gets returned and 'num_cpus' gets
> + * updated to reflect the intended number.
> + * - Otherwise, 'num_cpus' shall indicate the number of entries written, which
> + * may be less than the input value.
> */
> struct xen_sysctl_cputopoinfo {
> uint32_t num_cpus;
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-07-15 15:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-13 15:53 [PATCH] sysctl: adjust XEN_SYSCTL_cputopoinfo behavior Jan Beulich
2015-07-13 17:43 ` Boris Ostrovsky
2015-07-14 9:41 ` Jan Beulich
2015-07-15 15:51 ` Ian Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).