[PATCH 1/1] cpumask: add alloc_cpumask_var_node

[PATCH 1/1] cpumask: add alloc_cpumask_var_node

[Mike Travis]

Add alloc_cpumask_var_node.  This will be needed in x86 code to
allocate the domain and old_domain cpumasks on the same node as
where the containing irq_cfg struct is allocated.

Additional documentation added to all the alloc_cpumask and free_cpumask
functions.

For submission via linux-next.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 include/linux/cpumask.h |    1 
 lib/cpumask.c           |   60 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

--- linux-2.6-next.orig/include/linux/cpumask.h
+++ linux-2.6-next/include/linux/cpumask.h
@@ -1008,6 +1008,7 @@ static inline size_t cpumask_size(void)
 typedef struct cpumask *cpumask_var_t;
 
 bool alloc_cpumask_var(cpumask_var_t *mask, gfp_t flags);
+bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags, int node);
 void alloc_bootmem_cpumask_var(cpumask_var_t *mask);
 void free_cpumask_var(cpumask_var_t mask);
 void free_bootmem_cpumask_var(cpumask_var_t mask);
--- linux-2.6-next.orig/lib/cpumask.c
+++ linux-2.6-next/lib/cpumask.c
@@ -76,6 +76,15 @@ int cpumask_any_but(const struct cpumask
 
 /* These are not inline because of header tangles. */
 #ifdef CONFIG_CPUMASK_OFFSTACK
+/**
+ * alloc_cpumask_var - return an allocated struct cpumask
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ * @cpu: GFP_ flags
+ *
+ * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
+ * a nop returning a constant 1 (in <linux/cpumask.h>)
+ * Returns TRUE if memory allocation succeeded, FALSE otherwise.
+ */
 bool alloc_cpumask_var(cpumask_var_t *mask, gfp_t flags)
 {
 	if (likely(slab_is_available()))
@@ -98,17 +107,68 @@ bool alloc_cpumask_var(cpumask_var_t *ma
 }
 EXPORT_SYMBOL(alloc_cpumask_var);
 
+/**
+ * alloc_cpumask_var_node - return an allocated struct cpumask on a
+ * specific node.
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ * @cpu: GFP_ flags
+ * @node: node to allocate memory on
+ *
+ * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
+ * a nop returning a constant 1 (in <linux/cpumask.h>)
+ * Returns TRUE if memory allocation succeeded, FALSE otherwise.
+ */
+bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags, int node)
+{
+	if (likely(slab_is_available()))
+		*mask = kmalloc_node(cpumask_size(), flags, node);
+	else {
+#ifdef CONFIG_DEBUG_PER_CPU_MAPS
+		printk(KERN_ERR
+			"=> alloc_cpumask_var_node: kmalloc not available!\n");
+		dump_stack();
+#endif
+		*mask = NULL;
+	}
+#ifdef CONFIG_DEBUG_PER_CPU_MAPS
+	if (!*mask) {
+		printk(KERN_ERR "=> alloc_cpumask_var_node: failed!\n");
+		dump_stack();
+	}
+#endif
+	return *mask != NULL;
+}
+EXPORT_SYMBOL(alloc_cpumask_var);
+
+/**
+ * alloc_bootmem_cpumask_var - return an allocated struct cpumask from
+ * the bootmem arena.
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ *
+ * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
+ * a nop returning a constant 1 (in <linux/cpumask.h>)
+ * Either returns an allocated (zero-filled) cpumask, or causes the
+ * system to panic.
+ */
 void __init alloc_bootmem_cpumask_var(cpumask_var_t *mask)
 {
 	*mask = alloc_bootmem(cpumask_size());
 }
 
+/**
+ * free_cpumask_var - frees memory allocated for a struct cpumask.
+ * mask: cpumask to free
+ */
 void free_cpumask_var(cpumask_var_t mask)
 {
 	kfree(mask);
 }
 EXPORT_SYMBOL(free_cpumask_var);
 
+/**
+ * free_bootmem_cpumask_var - frees bootmem memory allocated for a struct cpumask.
+ * mask: cpumask to free
+ */
 void __init free_bootmem_cpumask_var(cpumask_var_t mask)
 {
 	free_bootmem((unsigned long)mask, cpumask_size());

Re: [PATCH 1/1] cpumask: add alloc_cpumask_var_node

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]

Hi Mike,

Just some simple comments.

On Mon, 15 Dec 2008 18:32:51 -0800 Mike Travis <travis@sgi.com> wrote:
>
> +/**
> + * alloc_cpumask_var - return an allocated struct cpumask
> + * @mask: pointer to cpumask_var_t where the cpumask is returned
> + * @cpu: GFP_ flags
       ^^^
flags

> + * alloc_cpumask_var_node - return an allocated struct cpumask on a
> + * specific node.
> + * @mask: pointer to cpumask_var_t where the cpumask is returned
> + * @cpu: GFP_ flags
       ^^^
flags

> + * @node: node to allocate memory on
> + *
> + * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
> + * a nop returning a constant 1 (in <linux/cpumask.h>)
> + * Returns TRUE if memory allocation succeeded, FALSE otherwise.
> + */
> +bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags, int node)
> +{
> +	if (likely(slab_is_available()))
> +		*mask = kmalloc_node(cpumask_size(), flags, node);
> +	else {
> +#ifdef CONFIG_DEBUG_PER_CPU_MAPS
> +		printk(KERN_ERR
> +			"=> alloc_cpumask_var_node: kmalloc not available!\n");
> +		dump_stack();
> +#endif
> +		*mask = NULL;
> +	}
> +#ifdef CONFIG_DEBUG_PER_CPU_MAPS
> +	if (!*mask) {
> +		printk(KERN_ERR "=> alloc_cpumask_var_node: failed!\n");
> +		dump_stack();
> +	}
> +#endif

So if !slab_is_available() and CONFIG_DEBUG_PER_CPU_MAPS is set, we get
two stack dumps?

> + * free_cpumask_var - frees memory allocated for a struct cpumask.
> + * mask: cpumask to free
      ^
missing '@'

> + * free_bootmem_cpumask_var - frees bootmem memory allocated for a struct cpumask.
> + * mask: cpumask to free
      ^
here as well.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/1] cpumask: add alloc_cpumask_var_node

[Mike Travis]

Stephen Rothwell wrote:
> Hi Mike,
> 
> Just some simple comments.
> 
> On Mon, 15 Dec 2008 18:32:51 -0800 Mike Travis <travis@sgi.com> wrote:
>> +/**
>> + * alloc_cpumask_var - return an allocated struct cpumask
>> + * @mask: pointer to cpumask_var_t where the cpumask is returned
>> + * @cpu: GFP_ flags
>        ^^^
> flags

Hmm, even the simplest changes can go awry... ;-)

> 
>> + * alloc_cpumask_var_node - return an allocated struct cpumask on a
>> + * specific node.
>> + * @mask: pointer to cpumask_var_t where the cpumask is returned
>> + * @cpu: GFP_ flags
>        ^^^
> flags

Gotcha.
> 
>> + * @node: node to allocate memory on
>> + *
>> + * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
>> + * a nop returning a constant 1 (in <linux/cpumask.h>)
>> + * Returns TRUE if memory allocation succeeded, FALSE otherwise.
>> + */
>> +bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags, int node)
>> +{
>> +	if (likely(slab_is_available()))
>> +		*mask = kmalloc_node(cpumask_size(), flags, node);
>> +	else {
>> +#ifdef CONFIG_DEBUG_PER_CPU_MAPS
>> +		printk(KERN_ERR
>> +			"=> alloc_cpumask_var_node: kmalloc not available!\n");
>> +		dump_stack();
>> +#endif
>> +		*mask = NULL;
>> +	}
>> +#ifdef CONFIG_DEBUG_PER_CPU_MAPS
>> +	if (!*mask) {
>> +		printk(KERN_ERR "=> alloc_cpumask_var_node: failed!\n");
>> +		dump_stack();
>> +	}
>> +#endif
> 
> So if !slab_is_available() and CONFIG_DEBUG_PER_CPU_MAPS is set, we get
> two stack dumps?

Umm, you're right, one message should suffice... ;-)

Here's one place I discovered it in testing:

[    0.000000] Initializing CPU#0
[    0.000000] RCU-based detection of stalled CPUs is enabled.
[    0.000000] => alloc_cpumask_var: kmalloc not available!
[    0.000000] Pid: 0, comm: swapper Not tainted 2.6.28-rc8-128-defconfig.12151554-00944-g968ea6d
-dirty #21
[    0.000000] Call Trace:
[    0.000000]  [<ffffffff8123b5bb>] alloc_cpumask_var+0x73/0xa7
[    0.000000]  [<ffffffff819197d0>] arch_early_irq_init+0x35/0xc3
[    0.000000]  [<ffffffff8192583d>] early_irq_init+0x57/0x59
[    0.000000]  [<ffffffff8190ab77>] start_kernel+0x1f9/0x3c2
[    0.000000]  [<ffffffff8190a29a>] x86_64_start_reservations+0xa9/0xad
[    0.000000]  [<ffffffff8190a39b>] x86_64_start_kernel+0xda/0xe1
[    0.000000] => alloc_cpumask_var: failed!
<same as above>
...

The message clued me into the fact that the function was allocating
early and needed to be an alloc_bootmem_cpumask_var().
                                                                                                 
> 
>> + * free_cpumask_var - frees memory allocated for a struct cpumask.
>> + * mask: cpumask to free
>       ^
> missing '@'
> 
>> + * free_bootmem_cpumask_var - frees bootmem memory allocated for a struct cpumask.
>> + * mask: cpumask to free
>       ^
> here as well.
> 

Will fix it up and resend.

Thanks!
Mike

[PATCH 1/1] cpumask: add alloc_cpumask_var_node V2

[Mike Travis]

Subject: cpumask: add alloc_cpumask_var_node V2

V2: Fix syntax errors in function comments.
    Display only one error message when !slab_is_available.
    Add nop inline alloc_cpumask_var_node() for !SMP,
    Fix EXPORT_SYMBOL for alloc_cpumask_var_node.

Add alloc_cpumask_var_node.  This will be needed in x86 code to
allocate the domain and old_domain cpumasks on the same node as
where the containing irq_cfg struct is allocated.

Additional documentation added to all the alloc_cpumask and free_cpumask
functions.

For submission via linux-next.

Signed-off-by: Mike Travis <travis@sgi.com>
---
 include/linux/cpumask.h |    7 ++++
 lib/cpumask.c           |   70 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 73 insertions(+), 4 deletions(-)

--- linux-2.6-for-ingo.orig/include/linux/cpumask.h
+++ linux-2.6-for-ingo/include/linux/cpumask.h
@@ -1026,6 +1026,7 @@ static inline size_t cpumask_size(void)
 typedef struct cpumask *cpumask_var_t;
 
 bool alloc_cpumask_var(cpumask_var_t *mask, gfp_t flags);
+bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags, int node);
 void alloc_bootmem_cpumask_var(cpumask_var_t *mask);
 void free_cpumask_var(cpumask_var_t mask);
 void free_bootmem_cpumask_var(cpumask_var_t mask);
@@ -1038,6 +1039,12 @@ static inline bool alloc_cpumask_var(cpu
 	return true;
 }
 
+static inline bool alloc_cpumask_var_node(cpumask_var_t *mask,
+					  gfp_t flags, int node)
+{
+	return true;
+}
+
 static inline void alloc_bootmem_cpumask_var(cpumask_var_t *mask)
 {
 }
--- linux-2.6-for-ingo.orig/lib/cpumask.c
+++ linux-2.6-for-ingo/lib/cpumask.c
@@ -76,39 +76,101 @@ int cpumask_any_but(const struct cpumask
 
 /* These are not inline because of header tangles. */
 #ifdef CONFIG_CPUMASK_OFFSTACK
+/**
+ * alloc_cpumask_var - return an allocated struct cpumask
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ * @flags: GFP_ flags
+ *
+ * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
+ * a nop returning a constant 1 (in <linux/cpumask.h>)
+ * Returns TRUE if memory allocation succeeded, FALSE otherwise.
+ */
 bool alloc_cpumask_var(cpumask_var_t *mask, gfp_t flags)
 {
 	if (likely(slab_is_available()))
 		*mask = kmalloc(cpumask_size(), flags);
 	else {
+		*mask = NULL;
+
 #ifdef CONFIG_DEBUG_PER_CPU_MAPS
 		printk(KERN_ERR
 			"=> alloc_cpumask_var: kmalloc not available!\n");
 		dump_stack();
-#endif
-		*mask = NULL;
+		return false;
 	}
-#ifdef CONFIG_DEBUG_PER_CPU_MAPS
 	if (!*mask) {
 		printk(KERN_ERR "=> alloc_cpumask_var: failed!\n");
 		dump_stack();
-	}
 #endif
+	}
+
 	return *mask != NULL;
 }
 EXPORT_SYMBOL(alloc_cpumask_var);
 
+/**
+ * alloc_cpumask_var_node - return an allocated struct cpumask on a
+ * specific node.
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ * @flags: GFP_ flags
+ * @node: node to allocate memory on
+ *
+ * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
+ * a nop returning a constant 1 (in <linux/cpumask.h>)
+ * Returns TRUE if memory allocation succeeded, FALSE otherwise.
+ */
+bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags, int node)
+{
+	if (likely(slab_is_available()))
+		*mask = kmalloc_node(cpumask_size(), flags, node);
+	else {
+		*mask = NULL;
+
+#ifdef CONFIG_DEBUG_PER_CPU_MAPS
+		printk(KERN_ERR
+			"=> alloc_cpumask_var_node: kmalloc not available!\n");
+		dump_stack();
+		return false;
+	}
+	if (!*mask) {
+		printk(KERN_ERR "=> alloc_cpumask_var_node: failed!\n");
+		dump_stack();
+#endif
+	}
+
+	return *mask != NULL;
+}
+EXPORT_SYMBOL(alloc_cpumask_var_node);
+
+/**
+ * alloc_bootmem_cpumask_var - return an allocated struct cpumask from
+ * the bootmem arena.
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ *
+ * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
+ * a nop returning a constant 1 (in <linux/cpumask.h>)
+ * Either returns an allocated (zero-filled) cpumask, or causes the
+ * system to panic.
+ */
 void __init alloc_bootmem_cpumask_var(cpumask_var_t *mask)
 {
 	*mask = alloc_bootmem(cpumask_size());
 }
 
+/**
+ * free_cpumask_var - frees memory allocated for a struct cpumask.
+ * @mask: cpumask to free
+ */
 void free_cpumask_var(cpumask_var_t mask)
 {
 	kfree(mask);
 }
 EXPORT_SYMBOL(free_cpumask_var);
 
+/**
+ * free_bootmem_cpumask_var - frees bootmem memory allocated for a struct cpumask.
+ * @mask: cpumask to free
+ */
 void __init free_bootmem_cpumask_var(cpumask_var_t mask)
 {
 	free_bootmem((unsigned long)mask, cpumask_size());

Re: [PATCH 1/1] cpumask: add alloc_cpumask_var_node V2

[Rusty Russell]

On Tuesday 16 December 2008 14:49:37 Mike Travis wrote:
> Add alloc_cpumask_var_node.  This will be needed in x86 code to
> allocate the domain and old_domain cpumasks on the same node as
> where the containing irq_cfg struct is allocated.

OK, this is understandable.  If you don't mind I'll split this patch
into documentation and node patches separately.

Since kmalloc(size, gfp) == kmalloc_node(size, gfp, -1), I think
we can change this to implement alloc_cpumask_var() in terms of
alloc_cpumask_var_node(..., numa_node_id()).

Here's what I ended up with:

Add alloc_cpumask_var_node()

This will be needed in x86 code to allocate the domain and old_domain
cpumasks on the same node as where the containing irq_cfg struct is
allocated.

Signed-off-by: Mike Travis <travis@sgi.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (re-impl alloc_cpumask_var)

diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -1011,6 +1011,7 @@ static inline size_t cpumask_size(void)
 #ifdef CONFIG_CPUMASK_OFFSTACK
 typedef struct cpumask *cpumask_var_t;
 
+bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags, int node);
 bool alloc_cpumask_var(cpumask_var_t *mask, gfp_t flags);
 void alloc_bootmem_cpumask_var(cpumask_var_t *mask);
 void free_cpumask_var(cpumask_var_t mask);
@@ -1020,6 +1021,12 @@ typedef struct cpumask cpumask_var_t[1];
 typedef struct cpumask cpumask_var_t[1];
 
 static inline bool alloc_cpumask_var(cpumask_var_t *mask, gfp_t flags)
+{
+	return true;
+}
+
+static inline bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags,
+					  int node)
 {
 	return true;
 }
diff --git a/lib/cpumask.c b/lib/cpumask.c
--- a/lib/cpumask.c
+++ b/lib/cpumask.c
@@ -76,15 +76,14 @@ int cpumask_any_but(const struct cpumask
 
 /* These are not inline because of header tangles. */
 #ifdef CONFIG_CPUMASK_OFFSTACK
-bool alloc_cpumask_var(cpumask_var_t *mask, gfp_t flags)
+bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags, int node)
 {
 	if (likely(slab_is_available()))
-		*mask = kmalloc(cpumask_size(), flags);
+		*mask = kmalloc_node(cpumask_size(), flags, node);
 	else {
 #ifdef CONFIG_DEBUG_PER_CPU_MAPS
 		printk(KERN_ERR
 			"=> alloc_cpumask_var: kmalloc not available!\n");
-		dump_stack();
 #endif
 		*mask = NULL;
 	}
@@ -95,6 +94,12 @@ bool alloc_cpumask_var(cpumask_var_t *ma
 	}
 #endif
 	return *mask != NULL;
+}
+EXPORT_SYMBOL(alloc_cpumask_var_node);
+
+bool alloc_cpumask_var(cpumask_var_t *mask, gfp_t flags)
+{
+	return alloc_cpumask_var_node(mask, flag, numa_node_id());
 }
 EXPORT_SYMBOL(alloc_cpumask_var);
 
cpumask: documentation for cpumask_var_t

Additional documentation added to all the alloc_cpumask and free_cpumask
functions.

Signed-off-by: Mike Travis <travis@sgi.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (minor additions)

diff --git a/lib/cpumask.c b/lib/cpumask.c
--- a/lib/cpumask.c
+++ b/lib/cpumask.c
@@ -76,6 +76,20 @@ int cpumask_any_but(const struct cpumask
 
 /* These are not inline because of header tangles. */
 #ifdef CONFIG_CPUMASK_OFFSTACK
+/**
+ * alloc_cpumask_var_node - allocate a struct cpumask on a given node
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ * @flags: GFP_ flags
+ *
+ * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
+ * a nop returning a constant 1 (in <linux/cpumask.h>)
+ * Returns TRUE if memory allocation succeeded, FALSE otherwise.
+ *
+ * In addition, mask will be NULL if this fails.  Note that gcc is
+ * usually smart enough to know that mask can never be NULL if
+ * CONFIG_CPUMASK_OFFSTACK=n, so does code elimination in that case
+ * too.
+ */
 bool alloc_cpumask_var_node(cpumask_var_t *mask, gfp_t flags, int node)
 {
 	if (likely(slab_is_available()))
@@ -97,23 +111,52 @@ bool alloc_cpumask_var_node(cpumask_var_
 }
 EXPORT_SYMBOL(alloc_cpumask_var_node);
 
+/**
+ * alloc_cpumask_var - allocate a struct cpumask
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ * @flags: GFP_ flags
+ *
+ * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
+ * a nop returning a constant 1 (in <linux/cpumask.h>).
+ *
+ * See alloc_cpumask_var_node.
+ */
 bool alloc_cpumask_var(cpumask_var_t *mask, gfp_t flags)
 {
 	return alloc_cpumask_var_node(mask, flag, numa_node_id());
 }
 EXPORT_SYMBOL(alloc_cpumask_var);
 
+/**
+ * alloc_bootmem_cpumask_var - allocate a struct cpumask from the bootmem arena.
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ *
+ * Only defined when CONFIG_CPUMASK_OFFSTACK=y, otherwise is
+ * a nop returning a constant 1 (in <linux/cpumask.h>)
+ * Either returns an allocated (zero-filled) cpumask, or causes the
+ * system to panic.
+ */
 void __init alloc_bootmem_cpumask_var(cpumask_var_t *mask)
 {
 	*mask = alloc_bootmem(cpumask_size());
 }
 
+/**
+ * free_cpumask_var - frees memory allocated for a struct cpumask.
+ * @mask: cpumask to free
+ *
+ * This is safe on a NULL mask.
+ */
 void free_cpumask_var(cpumask_var_t mask)
 {
 	kfree(mask);
 }
 EXPORT_SYMBOL(free_cpumask_var);
 
+/**
+ * free_bootmem_cpumask_var - frees result of alloc_bootmem_cpumask_var
+ * @mask: cpumask to free
+ */
 void __init free_bootmem_cpumask_var(cpumask_var_t mask)
 {
 	free_bootmem((unsigned long)mask, cpumask_size());

Re: [PATCH 1/1] cpumask: add alloc_cpumask_var_node V2

[Mike Travis]

Rusty Russell wrote:
> On Tuesday 16 December 2008 14:49:37 Mike Travis wrote:
>> Add alloc_cpumask_var_node.  This will be needed in x86 code to
>> allocate the domain and old_domain cpumasks on the same node as
>> where the containing irq_cfg struct is allocated.
> 
> OK, this is understandable.  If you don't mind I'll split this patch
> into documentation and node patches separately.
> 
> Since kmalloc(size, gfp) == kmalloc_node(size, gfp, -1), I think
> we can change this to implement alloc_cpumask_var() in terms of
> alloc_cpumask_var_node(..., numa_node_id()).
> 

Cool, thanks!

Mike