Skip to contentSkip to conversation

cpumask: add alloc_cpumask_var_node

PATCH In review Mike Travis 3 participants 6 messages Dec 16, 2008 – Dec 17, 2008

Subsystems (3)
Linux-next tree Library code Bitmap API
lore
Share & tools
lore

Mike Travis

author
#0 lore raw

PATCH 1/1 cpumask: add 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 <[email protected]>
---
 include/linux/cpumask.h |    1 
 lib/cpumask.c           |   60 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
include/linux/cpumask.h +1 -0
@@ -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);
lib/cpumask.c +60 -0

83 lines not on this page · raw message

#1 lore raw

Hi Mike,

Just some simple comments.

Quoted 5 lines · On Mon, 15 Dec 2008 18:32:51 -0800 Mike Travis <[email protected]> wrote:

+/**
+ * alloc_cpumask_var - return an allocated struct cpumask
+ * @mask: pointer to cpumask_var_t where the cpumask is returned
+ * @cpu: GFP_ flags

       ^^^

flags

Quoted 4 lines

+ * 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

Quoted 14 lines

+ * @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.
+ */

lib/cpumask.c +8 -0
+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");

lib/cpumask.c bool alloc_cpumask_var(cpumask_var_t *ma · line 138

Quoted 10 lines
lib/cpumask.c +10 -0
+		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?

Quoted 2 lines

+ * free_cpumask_var - frees memory allocated for a struct cpumask.
+ * mask: cpumask to free

      ^

missing '@'

Quoted 2 lines

+ * free_bootmem_cpumask_var - frees bootmem memory allocated for a struct cpumask.
+ * mask: cpumask to free

      ^

here as well.

Signature · 3 lines
Cheers,
Stephen Rothwell                    [email protected]
http://www.canb.auug.org.au/~sfr/
author
#2 lore raw
Quoted 11 lines · Stephen Rothwell

Hi Mike,

Just some simple comments.

Quoted 4 lines · On Mon, 15 Dec 2008 18:32:51 -0800 Mike Travis <[email protected]> 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... ;-)

Quoted 7 lines
Quoted 5 lines

+ * 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.

Quoted 20 lines
Quoted 20 lines

+ * @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.
+ */

lib/cpumask.c +13 -0
+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
Quoted 8 lines

lib/cpumask.c bool alloc_cpumask_var(cpumask_var_t *ma · line 138

Quoted 5 lines
lib/cpumask.c +5 -0
+	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().

Quoted 11 lines
Quoted 3 lines

+ * free_cpumask_var - frees memory allocated for a struct cpumask.
+ * mask: cpumask to free

      ^

missing '@'

Quoted 2 lines

+ * 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

author
#3 lore raw

PATCH 1/1 cpumask: add alloc_cpumask_var_node V2

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 <[email protected]>
---
 include/linux/cpumask.h |    7 ++++
 lib/cpumask.c           |   70 +++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 73 insertions(+), 4 deletions(-)
include/linux/cpumask.h +7 -0
@@ -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)
 {
 }
lib/cpumask.c +66 -4

105 lines not on this page · raw message

#4 lore raw

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

Quoted 3 lines · 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 <[email protected]>
Signed-off-by Rusty Russell <[email protected]> (re-impl alloc_cpumask_var)
include/linux/cpumask.h +7 -0
@@ -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;
 }
lib/cpumask.c +8 -3
@@ -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 <[email protected]>
Signed-off-by Rusty Russell <[email protected]> (minor additions)
lib/cpumask.c +43 -0

72 lines not on this page · raw message

author
#5 lore raw

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

Quoted 5 lines · Rusty Russell
Quoted 3 lines · 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.

Quoted 7 lines

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