KVM: use modern cpumask primitives, no cpumask_t on stack

PATCH In review Hollis Blanchard 3 participants 7 messages Dec 11, 2008 – Dec 21, 2008 1 message not in the archive

Subsystems (2)

KVM Library code

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Hollis Blanchard

author

2008-12-11 16:51 UTC #1 lore raw

Quoted 17 lines · Avi Kivity

virt/kvm/kvm_main.c +2 -6

index ba4275d..2d6ca79 100644
@@ -568,14 +570,17 @@ static bool make_all_cpus_request(struct kvm *kvm, unsigned int req)
 		if (test_and_set_bit(req, &vcpu->requests))
 			continue;
 		cpu = vcpu->cpu;
-		if (cpu != -1 && cpu != me)
-			cpu_set(cpu, cpus);
-	}
-	if (!cpus_empty(cpus)) {
-		smp_call_function_mask(cpus, ack_flush, NULL, 1);
-		called = true;
+		if (cpus != NULL && cpu != -1 && cpu != me)
+			cpumask_set_cpu(cpu, cpus);
 	}

virt/kvm/kvm_main.c static bool make_all_cpus_request(struct kvm *kvm, unsigned int req) · line 585

Quoted 10 lines

virt/kvm/kvm_main.c +7 -0

+	if (unlikely(cpus == NULL))
+		smp_call_function_many(cpu_online_mask, ack_flush, NULL, 1);
+	else if (!cpumask_empty(cpus))
+		smp_call_function_many(cpus, ack_flush, NULL, 1);
+	else
+		called = false;
 	put_cpu();
+	free_cpumask_var(cpus);
 	return called;
 }

This patch breaks uniprocessor builds, because smp_call_function_many() is only defined for CONFIG_SMP.

Avi, I think you should be able to build a PowerPC KVM kernel at this point? That would have caught this error. Rusty, could you ack the following:

cpumask: define smp_call_function_many() for non-SMP builds

Signed-off-by Hollis Blanchard <[email protected]>

include/linux/smp.h +7 -7

@@ -67,13 +67,6 @@ int smp_call_function(void(*func)(void *
 /* Deprecated: use smp_call_function_many() which uses a cpumask ptr. */
 int smp_call_function_mask(cpumask_t mask, void(*func)(void *info), void *info,
                                int wait);
-
-static inline void smp_call_function_many(const struct cpumask *mask,
-                                         void (*func)(void *info), void *info,
-                                         int wait)
-{
-       smp_call_function_mask(*mask, func, info, wait);
-}
 
 int smp_call_function_single(int cpuid, void (*func) (void *info), void *info,
                                int wait);

@@ -151,6 +144,13 @@ static inline void init_call_single_data
 }
 #endif /* !SMP */
 
+static inline void smp_call_function_many(const struct cpumask *mask,
+                                         void (*func)(void *info), void *info,
+                                         int wait)
+{
+       smp_call_function_mask(*mask, func, info, wait);
+}
+
 /*
  * smp_processor_id(): get the current CPU ID.
  *

Signature · 2 lines

Hollis Blanchard
IBM Linux Technology Center

Rusty Russell

2008-12-15 08:35 UTC #2 lore raw

Quoted 2 lines · On Friday 12 December 2008 03:21:24 Hollis Blanchard wrote:

This patch breaks uniprocessor builds, because smp_call_function_many() is only defined for CONFIG_SMP.

Good catch. I missed it because it's fixed as a side-effect of a later patch in my series (before I convert users).

Linus, can you please apply this fix?

Thanks,
Rusty.

Subject: Define smp_call_function_many for UP

Otherwise those using it in transition patches (eg. kvm) can't compile with CONFIG_SMP=n:

arch/x86/kvm/../../../virt/kvm/kvm_main.c: In function 'make_all_cpus_request':
arch/x86/kvm/../../../virt/kvm/kvm_main.c:380: error: implicit declaration of function 'smp_call_function_many'

Signed-off-by Rusty Russell <[email protected]>

include/linux/smp.h +2 -0

@@ -146,6 +147,8 @@ static inline void smp_send_reschedule(i
 })
 #define smp_call_function_mask(mask, func, info, wait) \
 			(up_smp_call_function(func, info))
+#define smp_call_function_many(mask, func, info, wait) \
+			(up_smp_call_function(func, info))
 static inline void init_call_single_data(void)
 {
 }

Rusty Russell

2008-12-18 22:10 UTC #3 lore raw

PATCH 1/1 Define smp_call_function_many for UP

Linus, please apply. Otherwise those using it in transition patches (eg. kvm) can't compile with CONFIG_SMP=n:

arch/x86/kvm/../../../virt/kvm/kvm_main.c: In function 'make_all_cpus_request':
arch/x86/kvm/../../../virt/kvm/kvm_main.c:380: error: implicit declaration of function 'smp_call_function_many'

Signed-off-by Rusty Russell <[email protected]>

---
 include/linux/smp.h |    2 ++
 1 file changed, 2 insertions(+)

include/linux/smp.h +2 -0

@@ -146,6 +147,8 @@ static inline void smp_send_reschedule(i
 })
 #define smp_call_function_mask(mask, func, info, wait) \
 			(up_smp_call_function(func, info))
+#define smp_call_function_many(mask, func, info, wait) \
+			(up_smp_call_function(func, info))
 static inline void init_call_single_data(void)
 {
 }

Linus Torvalds

2008-12-18 22:17 UTC #4 lore raw

Re: [PATCH] Define smp_call_function_many for UP

Quoted 3 lines · Rusty Russell

Linus, please apply. Otherwise those using it in transition patches (eg. kvm) can't compile with CONFIG_SMP=n:

Umm. I already applied this a long time ago. It's commit d2ff911882b6bc693d86ca9566daac70aacbb2b3

Linus

Rusty Russell

2008-12-19 05:44 UTC #5 lore raw

Re: [PATCH] Define smp_call_function_many for UP

Quoted 8 lines · On Friday 19 December 2008 08:46:37 Linus Torvalds wrote:

Quoted 3 lines · Rusty Russell
Linus, please apply. Otherwise those using it in transition patches (eg. kvm) can't compile with CONFIG_SMP=n:
Umm. I already applied this a long time ago. It's commit d2ff911882b6bc693d86ca9566daac70aacbb2b3

Wow, it's like Santa came early!

Thanks,
Rusty.

Linus Torvalds

2008-12-19 17:12 UTC #6 lore raw

Re: [PATCH] Define smp_call_function_many for UP

Quoted 7 lines · Rusty Russell

Quoted 3 lines · On Friday 19 December 2008 08:46:37 Linus Torvalds wrote:
Umm. I already applied this a long time ago. It's commit d2ff911882b6bc693d86ca9566daac70aacbb2b3
Wow, it's like Santa came early!

Christmas at Rusty's house:

Dreary.

Linus

Rusty Russell

2008-12-21 08:11 UTC #7 lore raw

Re: [PATCH] Define smp_call_function_many for UP

Quoted 3 lines · On Saturday 20 December 2008 03:33:26 Linus Torvalds wrote:

Christmas at Rusty's house:
Dreary.

Far from! Sure, it's Arabella's first Xmas, but a Very Kernel Christmas can be lots of fun. Why just this morning, we did this together! See if you can tell which bits are hers...

Subject: Decorate lib/rbtree.c
From: Rusty and Arabella Russell <[email protected]>

A slight enhancement to the comments in lib/rbtree.c.

Signed-off-by Rusty and Arabella Russell <[email protected]>

---
 lib/rbtree.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

lib/rbtree.c +3 -1

index 48499c2..7848186 100644
@@ -17,7 +17,8 @@
   along with this program; if not, write to the Free Software
   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-  linux/lib/rbtree.c
+  An excellent description of this algorithm can be found at:
+	http://en.wikipedia.org/wiki/Red-black_tree
 */
 
 #include <linux/rbtree.h>

@@ -291,6 +292,7 @@ EXPORT_SYMBOL(rb_erase);
 
 /*
  * This function returns the first node (in sort order) of the tree.
+ * ssdddsddddddddddddd
  */
 struct rb_node *rb_first(struct rb_root *root)
 {