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