Re: [RFC PATCH 1/3] kvm: keep track of which task is running a KVM vcpu
On 12/03/2010 04:16 PM, Rik van Riel wrote: On 12/03/2010 07:17 AM, Srivatsa Vaddagiri wrote: On Thu, Dec 02, 2010 at 02:43:24PM -0500, Rik van Riel wrote: mutex_lock(&vcpu->mutex); +vcpu->task = current; Shouldn't we grab reference to current task_struct before storing a pointer to it? That should not be needed, since current cannot go away without setting vcpu->task back to NULL in vcpu_put. However, you do reference vcpu->task from other contexts. So some sort of safe reference is needed. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] kvm: keep track of which task is running a KVM vcpu
On 12/03/2010 04:50 PM, Rik van Riel wrote: On 12/02/2010 08:18 PM, Chris Wright wrote: * Rik van Riel (r...@redhat.com) wrote: Keep track of which task is running a KVM vcpu. This helps us figure out later what task to wake up if we want to boost a vcpu that got preempted. Unfortunately there are no guarantees that the same task always keeps the same vcpu, so we can only track the task across a single "run" of the vcpu. So shouldn't it confine to KVM_RUN? The other vcpu_load callers aren't always a vcpu in a useful runnable state. Yeah, probably. If you want I can move the setting of vcpu->task to kvm_vcpu_ioctl. No need, it's not like something bad will happen. What I'd really like to see is a soft binding between a vcpu and its thread, so directed yield works even if we're in qemu while we were scheduled out. In fact it's not an unlikely pattern: spin_lock(&lock) ... writel(some_port_handled_by_qemu) ... spin_unlock(&lock) but that can wait. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] kvm: keep track of which task is running a KVM vcpu
* Rik van Riel (r...@redhat.com) wrote: > On 12/02/2010 08:18 PM, Chris Wright wrote: > >* Rik van Riel (r...@redhat.com) wrote: > >>Keep track of which task is running a KVM vcpu. This helps us > >>figure out later what task to wake up if we want to boost a > >>vcpu that got preempted. > >> > >>Unfortunately there are no guarantees that the same task > >>always keeps the same vcpu, so we can only track the task > >>across a single "run" of the vcpu. > > > >So shouldn't it confine to KVM_RUN? The other vcpu_load callers aren't > >always a vcpu in a useful runnable state. > > Yeah, probably. If you want I can move the setting of > vcpu->task to kvm_vcpu_ioctl. Or maybe setting in sched_out and unsetting in sched_in. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] kvm: keep track of which task is running a KVM vcpu
On 12/02/2010 08:18 PM, Chris Wright wrote: * Rik van Riel (r...@redhat.com) wrote: Keep track of which task is running a KVM vcpu. This helps us figure out later what task to wake up if we want to boost a vcpu that got preempted. Unfortunately there are no guarantees that the same task always keeps the same vcpu, so we can only track the task across a single "run" of the vcpu. So shouldn't it confine to KVM_RUN? The other vcpu_load callers aren't always a vcpu in a useful runnable state. Yeah, probably. If you want I can move the setting of vcpu->task to kvm_vcpu_ioctl. -- All rights reversed -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] kvm: keep track of which task is running a KVM vcpu
On 12/03/2010 07:17 AM, Srivatsa Vaddagiri wrote: On Thu, Dec 02, 2010 at 02:43:24PM -0500, Rik van Riel wrote: mutex_lock(&vcpu->mutex); + vcpu->task = current; Shouldn't we grab reference to current task_struct before storing a pointer to it? That should not be needed, since current cannot go away without setting vcpu->task back to NULL in vcpu_put. -- All rights reversed -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] kvm: keep track of which task is running a KVM vcpu
On Thu, Dec 02, 2010 at 02:43:24PM -0500, Rik van Riel wrote: > mutex_lock(&vcpu->mutex); > + vcpu->task = current; Shouldn't we grab reference to current task_struct before storing a pointer to it? - vatsa -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 1/3] kvm: keep track of which task is running a KVM vcpu
* Rik van Riel (r...@redhat.com) wrote: > Keep track of which task is running a KVM vcpu. This helps us > figure out later what task to wake up if we want to boost a > vcpu that got preempted. > > Unfortunately there are no guarantees that the same task > always keeps the same vcpu, so we can only track the task > across a single "run" of the vcpu. So shouldn't it confine to KVM_RUN? The other vcpu_load callers aren't always a vcpu in a useful runnable state. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 1/3] kvm: keep track of which task is running a KVM vcpu
Keep track of which task is running a KVM vcpu. This helps us
figure out later what task to wake up if we want to boost a
vcpu that got preempted.
Unfortunately there are no guarantees that the same task
always keeps the same vcpu, so we can only track the task
across a single "run" of the vcpu.
Signed-off-by: Rik van Riel
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5fbdb55..cb73a73 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -79,6 +79,7 @@ struct kvm_vcpu {
#endif
int vcpu_id;
struct mutex mutex;
+ struct task_struct *task;
int cpu;
struct kvm_run *run;
unsigned long requests;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 050577a..2bffa47 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -741,6 +741,7 @@ void vcpu_load(struct kvm_vcpu *vcpu)
int cpu;
mutex_lock(&vcpu->mutex);
+ vcpu->task = current;
cpu = get_cpu();
preempt_notifier_register(&vcpu->preempt_notifier);
kvm_arch_vcpu_load(vcpu, cpu);
@@ -749,6 +750,7 @@ void vcpu_load(struct kvm_vcpu *vcpu)
void vcpu_put(struct kvm_vcpu *vcpu)
{
+ vcpu->task = NULL;
preempt_disable();
kvm_arch_vcpu_put(vcpu);
preempt_notifier_unregister(&vcpu->preempt_notifier);
--
All rights reversed.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
