Thread (26 messages) 26 messages, 9 authors, 2015-08-17

Re: [patch -rt 1/2] KVM: use simple waitqueue for vcpu->wq

From: Steven Rostedt <rostedt@goodmis.org>
Date: 2015-01-20 18:16:20
Also in: kvm, lkml

On Tue, 20 Jan 2015 16:46:53 +1100
Paul Mackerras [off-list ref] wrote:
On Mon, Jan 19, 2015 at 12:41:00PM -0200, Marcelo Tosatti wrote:
quoted
On Fri, Jan 16, 2015 at 11:48:46AM -0500, Steven Rostedt wrote:
quoted
quoted
 static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
 {
-	DEFINE_WAIT(wait);
+	DEFINE_SWAITER(wait);
 
-	prepare_to_wait(&vc->wq, &wait, TASK_INTERRUPTIBLE);
+	swait_prepare(&vc->wq, &wait, TASK_INTERRUPTIBLE);
 	vc->vcore_state = VCORE_SLEEPING;
 	spin_unlock(&vc->lock);
 	schedule();
-	finish_wait(&vc->wq, &wait);
+	swait_finish(&vc->wq, &wait);
 	spin_lock(&vc->lock);
 	vc->vcore_state = VCORE_INACTIVE;
 }
@@ -1613,7 +1613,7 @@
 			kvmppc_create_dtl_entry(vcpu, vc);
 			kvmppc_start_thread(vcpu);
 		} else if (vc->vcore_state == VCORE_SLEEPING) {
-			wake_up(&vc->wq);
+			swait_wake(&vc->wq);
I notice everywhere you have a swait_wake_interruptible() but here. Is
there a reason why?

IIRC, Peter wants to make swait wakeup usage homogenous. That is, you
either sleep in an interruptible state, or you don't. You can't mix and
match it.
IIUC there is only one waiter on this waitqueue at any given time.

Paul is that correct?
Yes, that's right.  It's only the task that has taken the
responsibility for running the virtual core that would be waiting on
that wait queue.
Thanks Paul, but it still makes me nervious.

I'm actually wondering if we should just nuke the _interruptible()
version of swait. As it should only be all interruptible or all not
interruptible, that the swait_wake() should just do the wake up
regardless. In which case, swait_wake() is good enough. No need to have
different versions where people may think do something special.

Peter?

-- Steve
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help