From mboxrd@z Thu Jan  1 00:00:00 1970
From: Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH 1/3v2] kvm-s390: Implement the directed yield (diag 9c)
 hypervisor call for KVM
Date: Tue, 24 Apr 2012 16:06:38 +0300
Message-ID: <4F96A55E.1050703@redhat.com>
References: <4F96A021.4000704@de.ibm.com> <1335272114-64505-1-git-send-email-borntraeger@de.ibm.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: Marcelo Tossati <mtosatti@redhat.com>,
	Carsten Otte <cotte@de.ibm.com>,
	Alexander Graf <agraf@suse.de>,
	Jens Freimann <jfrei@linux.vnet.ibm.com>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	Martin Schwidefsky <schwidefsky@de.ibm.com>,
	Heinz Graalfs <graalfs@linux.vnet.ibm.com>,
	KVM <kvm@vger.kernel.org>, Konstantin Weitz <WEITZKON@de.ibm.com>
To: Christian Borntraeger <borntraeger@de.ibm.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:38005 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754402Ab2DXNHL (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 24 Apr 2012 09:07:11 -0400
In-Reply-To: <1335272114-64505-1-git-send-email-borntraeger@de.ibm.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On 04/24/2012 03:55 PM, Christian Borntraeger wrote:
> From: Konstantin Weitz <WEITZKON@de.ibm.com>
>
> This patch implements the directed yield hypercall found on other
> System z hypervisors. It delegates execution time to the virtual cpu
> specified in the instruction's parameter.
>
> Useful to avoid long spinlock waits in the guest.
>
>  
>  struct kvm_s390_io_info {
> diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> index a353f0e..022962b 100644
> --- a/arch/s390/kvm/diag.c
> +++ b/arch/s390/kvm/diag.c
> @@ -53,6 +53,45 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
>  	return 0;
>  }
>  
> +static int __diag_time_slice_end_directed(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm *kvm = vcpu->kvm;
> +	struct kvm_vcpu *tcpu;
> +	int tid;
> +	int i;
> +
> +	tid = vcpu->run->s.regs.gprs[(vcpu->arch.sie_block->ipa & 0xf0) >> 4];
> +	vcpu->stat.diagnose_9c++;
> +	VCPU_EVENT(vcpu, 5, "diag time slice end directed to %d", tid);
> +
> +	if (tid == vcpu->vcpu_id)
> +		return 0;
> +
> +	kvm_for_each_vcpu(i, tcpu, kvm) {
> +		if (tcpu->vcpu_id == tid) {
> +			struct task_struct *task = NULL;
> +			struct pid *pid;
> +			rcu_read_lock();
> +			pid = rcu_dereference(tcpu->pid);
> +			if (pid)
> +				task = get_pid_task(tcpu->pid, PIDTYPE_PID);
> +			rcu_read_unlock();
> +			if (!task)
> +				break;
> +			if (task->flags & PF_VCPU) {
> +				put_task_struct(task);
> +				break;
> +			}
> +			if (yield_to(task, 1)) {
> +				put_task_struct(task);
> +				return 0;
> +			}
> +			put_task_struct(task);
> +		}
> +	}
> +	return 0;
> +}
> +
>

I think the code will me more readable, and less obvious that is was
copied from kvm_vcpu_on_spin(), if you put all the processing outside
the loop, except for matching the vpu itself.

So the code reads

   find a vcpu
   obtain the task
   do the yield

instead of looking like you're doing the processing for every vcpu.  The
loop is just a slow lookup which might some day be replaced by a table
lookup.

-- 
error compiling committee.c: too many arguments to function