Mailing List Archive

[PATCH 02 of 10 v2] xen, libxc: introduce node maps and masks
Following suit from cpumap and cpumask implementations.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Juergen Gross <juergen.gross@ts.fujitsu.com>

diff --git a/tools/libxc/xc_misc.c b/tools/libxc/xc_misc.c
--- a/tools/libxc/xc_misc.c
+++ b/tools/libxc/xc_misc.c
@@ -54,6 +54,11 @@ int xc_get_cpumap_size(xc_interface *xch
return (xc_get_max_cpus(xch) + 7) / 8;
}

+int xc_get_nodemap_size(xc_interface *xch)
+{
+ return (xc_get_max_nodes(xch) + 7) / 8;
+}
+
xc_cpumap_t xc_cpumap_alloc(xc_interface *xch)
{
int sz;
@@ -64,6 +69,16 @@ xc_cpumap_t xc_cpumap_alloc(xc_interface
return calloc(1, sz);
}

+xc_nodemap_t xc_nodemap_alloc(xc_interface *xch)
+{
+ int sz;
+
+ sz = xc_get_nodemap_size(xch);
+ if (sz == 0)
+ return NULL;
+ return calloc(1, sz);
+}
+
int xc_readconsolering(xc_interface *xch,
char *buffer,
unsigned int *pnr_chars,
diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
--- a/tools/libxc/xenctrl.h
+++ b/tools/libxc/xenctrl.h
@@ -330,12 +330,20 @@ int xc_get_cpumap_size(xc_interface *xch
/* allocate a cpumap */
xc_cpumap_t xc_cpumap_alloc(xc_interface *xch);

- /*
+/*
* NODEMAP handling
*/
+typedef uint8_t *xc_nodemap_t;
+
/* return maximum number of NUMA nodes the hypervisor supports */
int xc_get_max_nodes(xc_interface *xch);

+/* return array size for nodemap */
+int xc_get_nodemap_size(xc_interface *xch);
+
+/* allocate a nodemap */
+xc_nodemap_t xc_nodemap_alloc(xc_interface *xch);
+
/*
* DOMAIN DEBUGGING FUNCTIONS
*/
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -117,6 +117,30 @@ int xenctl_bitmap_to_cpumask(cpumask_var
return err;
}

+int nodemask_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_nodemap,
+ const nodemask_t *nodemask)
+{
+ return bitmap_to_xenctl_bitmap(xenctl_nodemap, cpumask_bits(nodemask),
+ MAX_NUMNODES);
+}
+
+int xenctl_bitmap_to_nodemask(nodemask_t *nodemask,
+ const struct xenctl_bitmap *xenctl_nodemap)
+{
+ int err = 0;
+
+ if ( alloc_nodemask_var(nodemask) ) {
+ err = xenctl_bitmap_to_bitmap(nodes_addr(*nodemask), xenctl_nodemap,
+ MAX_NUMNODES);
+ if ( err )
+ free_nodemask_var(*nodemask);
+ }
+ else
+ err = -ENOMEM;
+
+ return err;
+}
+
static inline int is_free_domid(domid_t dom)
{
struct domain *d;
diff --git a/xen/include/xen/nodemask.h b/xen/include/xen/nodemask.h
--- a/xen/include/xen/nodemask.h
+++ b/xen/include/xen/nodemask.h
@@ -298,6 +298,53 @@ static inline int __nodemask_parse(const
}
#endif

+/*
+ * nodemask_var_t: struct nodemask for stack usage.
+ *
+ * See definition of cpumask_var_t in include/xen//cpumask.h.
+ */
+#if MAX_NUMNODES > 2 * BITS_PER_LONG
+#include <xen/xmalloc.h>
+
+typedef nodemask_t *nodemask_var_t;
+
+#define nr_nodemask_bits (BITS_TO_LONGS(MAX_NUMNODES) * BITS_PER_LONG)
+
+static inline bool_t alloc_nodemask_var(nodemask_var_t *mask)
+{
+ *(void **)mask = _xmalloc(nr_nodemask_bits / 8, sizeof(long));
+ return *mask != NULL;
+}
+
+static inline bool_t zalloc_nodemask_var(nodemask_var_t *mask)
+{
+ *(void **)mask = _xzalloc(nr_nodemask_bits / 8, sizeof(long));
+ return *mask != NULL;
+}
+
+static inline void free_nodemask_var(nodemask_var_t mask)
+{
+ xfree(mask);
+}
+#else
+typedef nodemask_t nodemask_var_t;
+
+static inline bool_t alloc_nodemask_var(nodemask_var_t *mask)
+{
+ return 1;
+}
+
+static inline bool_t zalloc_nodemask_var(nodemask_var_t *mask)
+{
+ nodes_clear(*mask);
+ return 1;
+}
+
+static inline void free_nodemask_var(nodemask_var_t mask)
+{
+}
+#endif
+
#if MAX_NUMNODES > 1
#define for_each_node_mask(node, mask) \
for ((node) = first_node(mask); \

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 02 of 10 v2] xen, libxc: introduce node maps and masks [ In reply to ]
>>> On 19.12.12 at 20:07, Dario Faggioli <dario.faggioli@citrix.com> wrote:
> --- a/xen/include/xen/nodemask.h
> +++ b/xen/include/xen/nodemask.h
> @@ -298,6 +298,53 @@ static inline int __nodemask_parse(const
> }
> #endif
>
> +/*
> + * nodemask_var_t: struct nodemask for stack usage.
> + *
> + * See definition of cpumask_var_t in include/xen//cpumask.h.
> + */
> +#if MAX_NUMNODES > 2 * BITS_PER_LONG

Is that case reasonable to expect?

Jan

> +#include <xen/xmalloc.h>
> +
> +typedef nodemask_t *nodemask_var_t;
> +
> +#define nr_nodemask_bits (BITS_TO_LONGS(MAX_NUMNODES) * BITS_PER_LONG)
> +
> +static inline bool_t alloc_nodemask_var(nodemask_var_t *mask)
> +{
> + *(void **)mask = _xmalloc(nr_nodemask_bits / 8, sizeof(long));
> + return *mask != NULL;
> +}
> +
> +static inline bool_t zalloc_nodemask_var(nodemask_var_t *mask)
> +{
> + *(void **)mask = _xzalloc(nr_nodemask_bits / 8, sizeof(long));
> + return *mask != NULL;
> +}
> +
> +static inline void free_nodemask_var(nodemask_var_t mask)
> +{
> + xfree(mask);
> +}
> +#else
> +typedef nodemask_t nodemask_var_t;
> +
> +static inline bool_t alloc_nodemask_var(nodemask_var_t *mask)
> +{
> + return 1;
> +}
> +
> +static inline bool_t zalloc_nodemask_var(nodemask_var_t *mask)
> +{
> + nodes_clear(*mask);
> + return 1;
> +}
> +
> +static inline void free_nodemask_var(nodemask_var_t mask)
> +{
> +}
> +#endif
> +
> #if MAX_NUMNODES > 1
> #define for_each_node_mask(node, mask) \
> for ((node) = first_node(mask); \




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 02 of 10 v2] xen, libxc: introduce node maps and masks [ In reply to ]
On Thu, 2012-12-20 at 09:18 +0000, Jan Beulich wrote:
> > +/*
> > + * nodemask_var_t: struct nodemask for stack usage.
> > + *
> > + * See definition of cpumask_var_t in include/xen//cpumask.h.
> > + */
> > +#if MAX_NUMNODES > 2 * BITS_PER_LONG
>
> Is that case reasonable to expect?
>
I really don't think so. Here, I really was just aligning cpumask and
nodemask types. But you're right, thinking more about it, this is
completely unnecessary. I'll kill this hunk.

Thanks and Regards,
Dario

--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)
Re: [PATCH 02 of 10 v2] xen, libxc: introduce node maps and masks [ In reply to ]
On 20/12/12 09:18, Jan Beulich wrote:
>>>> On 19.12.12 at 20:07, Dario Faggioli <dario.faggioli@citrix.com> wrote:
>> --- a/xen/include/xen/nodemask.h
>> +++ b/xen/include/xen/nodemask.h
>> @@ -298,6 +298,53 @@ static inline int __nodemask_parse(const
>> }
>> #endif
>>
>> +/*
>> + * nodemask_var_t: struct nodemask for stack usage.
>> + *
>> + * See definition of cpumask_var_t in include/xen//cpumask.h.
>> + */
>> +#if MAX_NUMNODES > 2 * BITS_PER_LONG
> Is that case reasonable to expect?

2 * BITS_PER_LONG is just going to be 128, right? It wasn't too long
ago that I would have considered 4096 cores a pretty unreasonable
expectation. Is there a particular reason you think this is going to be
more than a few years away, and a particular harm in having the code
here to begin with?

At very least it should be replaced with something like this:

#if MAX_NUMNODES > 2 * BITS_PER_LONG
# error "MAX_NUMNODES exceeds fixed size nodemask; need to implement
variable-length nodemasks"
#endif

-George


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 02 of 10 v2] xen, libxc: introduce node maps and masks [ In reply to ]
>>> On 20.12.12 at 15:33, George Dunlap <george.dunlap@eu.citrix.com> wrote:
> On 20/12/12 09:18, Jan Beulich wrote:
>>>>> On 19.12.12 at 20:07, Dario Faggioli <dario.faggioli@citrix.com> wrote:
>>> --- a/xen/include/xen/nodemask.h
>>> +++ b/xen/include/xen/nodemask.h
>>> @@ -298,6 +298,53 @@ static inline int __nodemask_parse(const
>>> }
>>> #endif
>>>
>>> +/*
>>> + * nodemask_var_t: struct nodemask for stack usage.
>>> + *
>>> + * See definition of cpumask_var_t in include/xen//cpumask.h.
>>> + */
>>> +#if MAX_NUMNODES > 2 * BITS_PER_LONG
>> Is that case reasonable to expect?
>
> 2 * BITS_PER_LONG is just going to be 128, right? It wasn't too long
> ago that I would have considered 4096 cores a pretty unreasonable
> expectation. Is there a particular reason you think this is going to be
> more than a few years away, and a particular harm in having the code
> here to begin with?

I just don't see node counts grow even near as quickly as core/
thread ones.

> At very least it should be replaced with something like this:
>
> #if MAX_NUMNODES > 2 * BITS_PER_LONG
> # error "MAX_NUMNODES exceeds fixed size nodemask; need to implement
> variable-length nodemasks"
> #endif

Yes, if there is a limitation in the code, it being violated should be
detected at build time. But I'd suppose on can construct the
statically sized mask definition such that it copes with larger counts
(just at the expense of having larger data objects perhaps
on-stack). Making sure to always pass pointers rather than objects
to functions will already eliminated a good part of the problem.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [PATCH 02 of 10 v2] xen, libxc: introduce node maps and masks [ In reply to ]
On Thu, Dec 20, 2012 at 3:52 PM, Jan Beulich <JBeulich@suse.com> wrote:
>> 2 * BITS_PER_LONG is just going to be 128, right? It wasn't too long
>> ago that I would have considered 4096 cores a pretty unreasonable
>> expectation. Is there a particular reason you think this is going to be
>> more than a few years away, and a particular harm in having the code
>> here to begin with?
>
> I just don't see node counts grow even near as quickly as core/
> thread ones.
>
Yep, same here, that's why, despite having put that code there, I agree
with Jan in removing it. :-)

That feeling matches with what we've been repeatedly told by hardware
vendors, about node count already saturating at around 8. Beyond that, a
different architectural approach will be needed to tackle scalability issues
(clusters? NoCs?).

Just for the records, Linux still deals with this in the same way it did when
we took these files from there (i.e., without this hunk):
- cpumasks exists in static and dynamically allocated (cpumask_var_t) foms
- nodemask are only and always statically allocated

Dario

--
<<This happens because I choose it to happen!>> (Raistlin Majere)
---------------------------------------------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel