Re: [GIT PULL] First batch of KVM changes for 4.1

[Paolo Bonzini <pbonzini@redhat.com>]

On 17/04/2015 12:55, Peter Zijlstra wrote:
> Also, it looks like you already do exactly this for other things, look
> at:
> 
> 	kvm_sched_in()
> 	  kvm_arch_vcpu_load()
> 	    if (unlikely(vcpu->cpu != cpu) ... )
> 
> So no, I don't believe for one second you need this.

You're missing that this snippet is running in the host, while this 
patch is concerned with the guest (paravirt).

This notifier runs for _all_ tasks, not just for the KVM threads.  In 
fact there will most likely be no KVM in the guest.

There is no vcpu->cpu where this notifier is run.

And frankly, I think the static key is snake oil.  The cost of task 
migration in terms of cache misses and TLB misses is in no way 
comparable to the cost of filling in a structure on the stack, 
dereferencing the head of the notifiers list and seeing that it's NULL.

If this was a real problem, it would be better solved by adding 
inlining in kernel/notifier.c:

diff --git a/kernel/notifier.c b/kernel/notifier.c
index ae9fc7cc360e..9c0cd0f739e0 100644
--- a/kernel/notifier.c
+++ b/kernel/notifier.c
@@ -59,28 +59,14 @@ static int notifier_chain_unregister(struct notifier_block **nl,
 	return -ENOENT;
 }
 
-/**
- * notifier_call_chain - Informs the registered notifiers about an event.
- *	@nl:		Pointer to head of the blocking notifier chain
- *	@val:		Value passed unmodified to notifier function
- *	@v:		Pointer passed unmodified to notifier function
- *	@nr_to_call:	Number of notifier functions to be called. Don't care
- *			value of this parameter is -1.
- *	@nr_calls:	Records the number of notifications sent. Don't care
- *			value of this field is NULL.
- *	@returns:	notifier_call_chain returns the value returned by the
- *			last notifier function called.
- */
-static int notifier_call_chain(struct notifier_block **nl,
-			       unsigned long val, void *v,
-			       int nr_to_call, int *nr_calls)
+static int __notifier_call_chain(struct notifier_block *nb,
+			         unsigned long val, void *v,
+			         int nr_to_call, int *nr_calls)
 {
 	int ret = NOTIFY_DONE;
-	struct notifier_block *nb, *next_nb;
-
-	nb = rcu_dereference_raw(*nl);
+	struct notifier_block *next_nb;
 
-	while (nb && nr_to_call) {
+	do {
 		next_nb = rcu_dereference_raw(nb->next);
 
 #ifdef CONFIG_DEBUG_NOTIFIERS
@@ -94,14 +80,38 @@ static int notifier_call_chain(struct notifier_block **nl,
 
 		if (nr_calls)
 			(*nr_calls)++;
-
-		if ((ret & NOTIFY_STOP_MASK) == NOTIFY_STOP_MASK)
-			break;
-		nb = next_nb;
-		nr_to_call--;
-	}
+	} while (!(ret & NOTIFY_STOP_MASK) &&
+		 (nb = next_nb) != NULL &&
+		 --nr_to_call);
 	return ret;
 }
+NOKPROBE_SYMBOL(__notifier_call_chain);
+
+/**
+ * notifier_call_chain - Informs the registered notifiers about an event.
+ *	@nl:		Pointer to head of the blocking notifier chain
+ *	@val:		Value passed unmodified to notifier function
+ *	@v:		Pointer passed unmodified to notifier function
+ *	@nr_to_call:	Number of notifier functions to be called. Don't care
+ *			value of this parameter is -1.
+ *	@nr_calls:	Records the number of notifications sent. Don't care
+ *			value of this field is NULL.
+ *	@returns:	notifier_call_chain returns the value returned by the
+ *			last notifier function called.
+ */
+static __always_inline int notifier_call_chain(struct notifier_block **nl,
+			      		       unsigned long val, void *v,
+					       int nr_to_call, int *nr_calls)
+{
+	struct notifier_block *nb = rcu_dereference_raw(*nl);
+	if (unlikely(nr_to_call == 0))
+		return NOTIFY_DONE;
+
+	if (!nb)
+		return NOTIFY_DONE;
+
+	return __notifier_call_chain(nb, val, v, nr_to_call, nr_calls);
+}
 NOKPROBE_SYMBOL(notifier_call_chain);
 
 /*
@@ -190,7 +199,12 @@ NOKPROBE_SYMBOL(__atomic_notifier_call_chain);
 int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
 			       unsigned long val, void *v)
 {
-	return __atomic_notifier_call_chain(nh, val, v, -1, NULL);
+	int ret;
+
+	rcu_read_lock();
+	ret = notifier_call_chain(&nh->head, val, v, -1, NULL);
+	rcu_read_unlock();
+	return ret;
 }
 EXPORT_SYMBOL_GPL(atomic_notifier_call_chain);
 NOKPROBE_SYMBOL(atomic_notifier_call_chain);


Also, move enough stuff to a header so that the fast path is inlined to
a single pointer derefrence.

Paolo