[PATCH 0/3] Fixes and feature for kvm-s390

[PATCH 0/3] Fixes and feature for kvm-s390

[Christian Borntraeger]

Avi, Marcelo,

here are 3 patches for kvm on s390.
One patch implements diag 9c (directed yield).
Tthe two other patches implement a missing instruction and
KVM_CAP_MAX_VCPUS.

ALl 3 are targeted for the next merge window.

Thanks

Christian

Christian Borntraeger (1):
  kvm-s390: implement KVM_CAP_MAX_VCPUS

Cornelia Huck (1):
  kvm-s390: Handle sckpf instruction

Konstantin Weitz (1):
  kvm-s390: Implement the directed yield (diag 9c) hypervisor call for
    KVM

 arch/s390/include/asm/kvm_host.h |    1 +
 arch/s390/kvm/diag.c             |   44 ++++++++++++++++++++++++++++++++++++++
 arch/s390/kvm/intercept.c        |    1 +
 arch/s390/kvm/kvm-s390.c         |    4 ++++
 arch/s390/kvm/kvm-s390.h         |    1 +
 arch/s390/kvm/priv.c             |   31 +++++++++++++++++++++++++++
 6 files changed, 82 insertions(+)

-- 
1.7.9.6

[PATCH 1/3] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Christian Borntraeger]

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.

Signed-off-by: Konstantin Weitz <WEITZKON@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |    1 +
 arch/s390/kvm/diag.c             |   44 ++++++++++++++++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.c         |    1 +
 3 files changed, 46 insertions(+)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 7343872..dd17537 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -148,6 +148,7 @@ struct kvm_vcpu_stat {
 	u32 instruction_sigp_restart;
 	u32 diagnose_10;
 	u32 diagnose_44;
+	u32 diagnose_9c;
 };
 
 struct kvm_s390_io_info {
diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index a353f0e..8991009 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -53,6 +53,48 @@ 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;
+			}
+			vcpu_put(vcpu);
+			if (yield_to(task, 1)) {
+				vcpu_load(vcpu);
+				put_task_struct(task);
+				return 0;
+			}
+			vcpu_load(vcpu);
+			put_task_struct(task);
+		}
+	}
+	return 0;
+}
+
 static int __diag_ipl_functions(struct kvm_vcpu *vcpu)
 {
 	unsigned int reg = vcpu->arch.sie_block->ipa & 0xf;
@@ -89,6 +131,8 @@ int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)
 		return diag_release_pages(vcpu);
 	case 0x44:
 		return __diag_time_slice_end(vcpu);
+	case 0x9c:
+		return __diag_time_slice_end_directed(vcpu);
 	case 0x308:
 		return __diag_ipl_functions(vcpu);
 	default:
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index d30c835..fd98914 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -74,6 +74,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	{ "instruction_sigp_restart", VCPU_STAT(instruction_sigp_restart) },
 	{ "diagnose_10", VCPU_STAT(diagnose_10) },
 	{ "diagnose_44", VCPU_STAT(diagnose_44) },
+	{ "diagnose_9c", VCPU_STAT(diagnose_9c) },
 	{ NULL }
 };
 
-- 
1.7.9.6

[PATCH 2/3] kvm-s390: Handle sckpf instruction

[Christian Borntraeger]

From: Cornelia Huck <cornelia.huck@de.ibm.com>

Handle the mandatory intercept SET CLOCK PROGRAMMABLE FIELD
instruction.

Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
 arch/s390/kvm/intercept.c |    1 +
 arch/s390/kvm/kvm-s390.h  |    1 +
 arch/s390/kvm/priv.c      |   31 +++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+)

diff --git a/arch/s390/kvm/intercept.c b/arch/s390/kvm/intercept.c
index 3614565..979cbe5 100644
--- a/arch/s390/kvm/intercept.c
+++ b/arch/s390/kvm/intercept.c
@@ -101,6 +101,7 @@ static int handle_lctl(struct kvm_vcpu *vcpu)
 }
 
 static intercept_handler_t instruction_handlers[256] = {
+	[0x01] = kvm_s390_handle_01,
 	[0x83] = kvm_s390_handle_diag,
 	[0xae] = kvm_s390_handle_sigp,
 	[0xb2] = kvm_s390_handle_b2,
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index ff28f9d..2294377 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -79,6 +79,7 @@ int kvm_s390_inject_sigp_stop(struct kvm_vcpu *vcpu, int action);
 /* implemented in priv.c */
 int kvm_s390_handle_b2(struct kvm_vcpu *vcpu);
 int kvm_s390_handle_e5(struct kvm_vcpu *vcpu);
+int kvm_s390_handle_01(struct kvm_vcpu *vcpu);
 
 /* implemented in sigp.c */
 int kvm_s390_handle_sigp(struct kvm_vcpu *vcpu);
diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
index e5a45db..68a6b2e 100644
--- a/arch/s390/kvm/priv.c
+++ b/arch/s390/kvm/priv.c
@@ -380,3 +380,34 @@ int kvm_s390_handle_e5(struct kvm_vcpu *vcpu)
 	return -EOPNOTSUPP;
 }
 
+static int handle_sckpf(struct kvm_vcpu *vcpu)
+{
+	u32 value;
+
+	if (vcpu->arch.sie_block->gpsw.mask & PSW_MASK_PSTATE)
+		return kvm_s390_inject_program_int(vcpu,
+						   PGM_PRIVILEGED_OPERATION);
+
+	if (vcpu->run->s.regs.gprs[0] & 0x00000000ffff0000)
+		return kvm_s390_inject_program_int(vcpu,
+						   PGM_SPECIFICATION);
+
+	value = vcpu->run->s.regs.gprs[0] & 0x000000000000ffff;
+	vcpu->arch.sie_block->todpr = value;
+
+	return 0;
+}
+
+static intercept_handler_t x01_handlers[256] = {
+	[0x07] = handle_sckpf,
+};
+
+int kvm_s390_handle_01(struct kvm_vcpu *vcpu)
+{
+	intercept_handler_t handler;
+
+	handler = x01_handlers[vcpu->arch.sie_block->ipa & 0x00ff];
+	if (handler)
+		return handler(vcpu);
+	return -EOPNOTSUPP;
+}
-- 
1.7.9.6

[PATCH 3/3] kvm-s390: implement KVM_CAP_MAX_VCPUS

[Christian Borntraeger]

Let userspace know the number of supported cpus fro kvm.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/kvm-s390.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index fd98914..0054cb0 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -136,6 +136,9 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_SYNC_REGS:
 		r = 1;
 		break;
+	case KVM_CAP_NR_VCPUS:
+		r = KVM_MAX_VCPUS;
+		break;
 	default:
 		r = 0;
 	}
-- 
1.7.9.6

Re: [PATCH 1/3] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Avi Kivity]

On 04/24/2012 10:24 AM, 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.
>
>
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 7343872..dd17537 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -148,6 +148,7 @@ struct kvm_vcpu_stat {
>  	u32 instruction_sigp_restart;
>  	u32 diagnose_10;
>  	u32 diagnose_44;
> +	u32 diagnose_9c;
>  };
>  
>  struct kvm_s390_io_info {
> diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> index a353f0e..8991009 100644
> --- a/arch/s390/kvm/diag.c
> +++ b/arch/s390/kvm/diag.c
> @@ -53,6 +53,48 @@ 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) {

Can two vcpus match the same 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;
> +			}
> +			vcpu_put(vcpu);
> +			if (yield_to(task, 1)) {
> +				vcpu_load(vcpu);

vcpu_put()/_load()s are unneeded here, preempt notifiers will
automatically call kvm_arch_vcpu_put()/_load() if we schedule.

> +				put_task_struct(task);
> +				return 0;
> +			}
> +			vcpu_load(vcpu);
> +			put_task_struct(task);
> +		}
> +	}
> +	return 0;
> +}
> +
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index d30c835..fd98914 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -74,6 +74,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>  	{ "instruction_sigp_restart", VCPU_STAT(instruction_sigp_restart) },
>  	{ "diagnose_10", VCPU_STAT(diagnose_10) },
>  	{ "diagnose_44", VCPU_STAT(diagnose_44) },
> +	{ "diagnose_9c", VCPU_STAT(diagnose_9c) },
>  	{ NULL }
>  };
>  

We're switching to tracepoints instead of homebrewed statistics.  It's
in feature-removal-schedule.txt already.  It's okay to add them now, but
please prepare for their removal.

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

Re: [PATCH 3/3] kvm-s390: implement KVM_CAP_MAX_VCPUS

[Avi Kivity]

On 04/24/2012 10:24 AM, Christian Borntraeger wrote:
> Let userspace know the number of supported cpus fro kvm.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c |    3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index fd98914..0054cb0 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -136,6 +136,9 @@ int kvm_dev_ioctl_check_extension(long ext)
>  	case KVM_CAP_SYNC_REGS:
>  		r = 1;
>  		break;
> +	case KVM_CAP_NR_VCPUS:
> +		r = KVM_MAX_VCPUS;
> +		break;
>  	default:
>  		r = 0;
>  	}

This looks like a bugfix, but having survived so long without it, I
imagine it's not urgent for 3.4.

btw, a better place might be in kvm_devel_ioctl_check_extension_generic().

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

Re: [PATCH 1/3] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Christian Borntraeger]

On 24/04/12 14:04, Avi Kivity wrote:
[...]

>> +	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) {
> 
> Can two vcpus match the same tid?

Dont think so. tid (target id) is the cpu number of the CPU holding the
spinlock and vcpu_id is checked for in kvm_vcpu_init to be unique.

[...]
>> +			vcpu_put(vcpu);
>> +			if (yield_to(task, 1)) {
>> +				vcpu_load(vcpu);
> 
> vcpu_put()/_load()s are unneeded here, preempt notifiers will
> automatically call kvm_arch_vcpu_put()/_load() if we schedule.

Right. Will send an updated patch.

[...]
>> @@ -74,6 +74,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>>  	{ "instruction_sigp_restart", VCPU_STAT(instruction_sigp_restart) },
>>  	{ "diagnose_10", VCPU_STAT(diagnose_10) },
>>  	{ "diagnose_44", VCPU_STAT(diagnose_44) },
>> +	{ "diagnose_9c", VCPU_STAT(diagnose_9c) },
>>  	{ NULL }
>>  };
>>  
> 
> We're switching to tracepoints instead of homebrewed statistics.  It's
> in feature-removal-schedule.txt already.  It's okay to add them now, but
> please prepare for their removal.

Yes, we have started with some tracepoint code conversion. We will try to speed
up a bit.

Christian

Re: [PATCH 3/3] kvm-s390: implement KVM_CAP_MAX_VCPUS

[Christian Borntraeger]

>> @@ -136,6 +136,9 @@ int kvm_dev_ioctl_check_extension(long ext)
>>  	case KVM_CAP_SYNC_REGS:
>>  		r = 1;
>>  		break;
>> +	case KVM_CAP_NR_VCPUS:
>> +		r = KVM_MAX_VCPUS;
>> +		break;
>>  	default:
>>  		r = 0;
>>  	}
> 
> This looks like a bugfix, but having survived so long without it, I
> imagine it's not urgent for 3.4.

Right it is not urgent.

> btw, a better place might be in kvm_devel_ioctl_check_extension_generic().

Possibly yes. Dont know if there are architecture specific reasons to use
something different than KVM_MAX_VCPUS.

[PATCH 1/3v2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Christian Borntraeger]

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.

Signed-off-by: Konstantin Weitz <WEITZKON@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |    1 +
 arch/s390/kvm/diag.c             |   41 ++++++++++++++++++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.c         |    1 +
 3 files changed, 43 insertions(+)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 7343872..dd17537 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -148,6 +148,7 @@ struct kvm_vcpu_stat {
 	u32 instruction_sigp_restart;
 	u32 diagnose_10;
 	u32 diagnose_44;
+	u32 diagnose_9c;
 };
 
 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;
+}
+
 static int __diag_ipl_functions(struct kvm_vcpu *vcpu)
 {
 	unsigned int reg = vcpu->arch.sie_block->ipa & 0xf;
@@ -89,6 +128,8 @@ int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)
 		return diag_release_pages(vcpu);
 	case 0x44:
 		return __diag_time_slice_end(vcpu);
+	case 0x9c:
+		return __diag_time_slice_end_directed(vcpu);
 	case 0x308:
 		return __diag_ipl_functions(vcpu);
 	default:
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index d30c835..fd98914 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -74,6 +74,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	{ "instruction_sigp_restart", VCPU_STAT(instruction_sigp_restart) },
 	{ "diagnose_10", VCPU_STAT(diagnose_10) },
 	{ "diagnose_44", VCPU_STAT(diagnose_44) },
+	{ "diagnose_9c", VCPU_STAT(diagnose_9c) },
 	{ NULL }
 };
 
-- 
1.7.9.6

Re: [PATCH 3/3] kvm-s390: implement KVM_CAP_MAX_VCPUS

[Avi Kivity]

On 04/24/2012 03:47 PM, Christian Borntraeger wrote:
> > btw, a better place might be in kvm_devel_ioctl_check_extension_generic().
>
> Possibly yes. Dont know if there are architecture specific reasons to use
> something different than KVM_MAX_VCPUS.

Okay, I'll fold the implementations unless something prevents it (unlikely).

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

Re: [PATCH 1/3v2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Avi Kivity]

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

Re: [PATCH 1/3v2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Christian Borntraeger]

> 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.

Ok. We might also have a kvm_vcpu_on_spin_directed in common code,
Would you prefer that? 

Christian

Re: [PATCH 1/3v2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Avi Kivity]

On 04/24/2012 06:47 PM, Christian Borntraeger wrote:
> > 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.
>
> Ok. We might also have a kvm_vcpu_on_spin_directed in common code,
> Would you prefer that? 

You mean a function that takes a vcpu and tries to yield_to() it?  Or
something else?

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

Re: [PATCH 1/3v2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Christian Borntraeger]

On 24/04/12 18:15, Avi Kivity wrote:
>>> 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.
>>
>> Ok. We might also have a kvm_vcpu_on_spin_directed in common code,
>> Would you prefer that? 
> 
> You mean a function that takes a vcpu and tries to yield_to() it?  Or
> something else?

Or a cpu id (number). But yes, something like that.

I dont know if that will be of use for x86 (e.g. parvirtual 
ticketlocks or something like that).

Christian

Re: [PATCH 1/3v2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Avi Kivity]

On 04/24/2012 07:21 PM, Christian Borntraeger wrote:
> On 24/04/12 18:15, Avi Kivity wrote:
> >>> 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.
> >>
> >> Ok. We might also have a kvm_vcpu_on_spin_directed in common code,
> >> Would you prefer that? 
> > 
> > You mean a function that takes a vcpu and tries to yield_to() it?  Or
> > something else?
>
> Or a cpu id (number). But yes, something like that.
>
> I dont know if that will be of use for x86 (e.g. parvirtual 
> ticketlocks or something like that).
>
>

It looks like kvm_vcpu_on_spin() can use it.

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

[PATCH 0/2] rework kvm_vcpu_on_spin

[Christian Borntraeger]

Avi,

what about this approach?

Christian Borntraeger (1):
  kvm-s390: use kvm_vcpu_on_spin for diag 0x44

Konstantin Weitz (1):
  kvm-s390: Implement the directed yield (diag 9c) hypervisor call for
    KVM

 arch/s390/include/asm/kvm_host.h |    1 +
 arch/s390/kvm/diag.c             |   29 +++++++++++++++++++++++---
 arch/s390/kvm/kvm-s390.c         |    1 +
 include/linux/kvm_host.h         |    1 +
 virt/kvm/kvm_main.c              |   42 +++++++++++++++++++++++---------------
 5 files changed, 55 insertions(+), 19 deletions(-)

-- 
1.7.9.6

[PATCH 1/2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Christian Borntraeger]

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.

Christian Borntraeger: moved common code in virt/kvm/

Signed-off-by: Konstantin Weitz <WEITZKON@de.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/include/asm/kvm_host.h |    1 +
 arch/s390/kvm/diag.c             |   25 +++++++++++++++++++++++
 arch/s390/kvm/kvm-s390.c         |    1 +
 include/linux/kvm_host.h         |    1 +
 virt/kvm/kvm_main.c              |   42 +++++++++++++++++++++++---------------
 5 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 7343872..dd17537 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -148,6 +148,7 @@ struct kvm_vcpu_stat {
 	u32 instruction_sigp_restart;
 	u32 diagnose_10;
 	u32 diagnose_44;
+	u32 diagnose_9c;
 };
 
 struct kvm_s390_io_info {
diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index a353f0e..2d2ae32 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -53,6 +53,29 @@ 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) {
+			kvm_vcpu_yield_to(tcpu);
+			break;
+		}
+
+	return 0;
+}
+
 static int __diag_ipl_functions(struct kvm_vcpu *vcpu)
 {
 	unsigned int reg = vcpu->arch.sie_block->ipa & 0xf;
@@ -89,6 +112,8 @@ int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)
 		return diag_release_pages(vcpu);
 	case 0x44:
 		return __diag_time_slice_end(vcpu);
+	case 0x9c:
+		return __diag_time_slice_end_directed(vcpu);
 	case 0x308:
 		return __diag_ipl_functions(vcpu);
 	default:
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 044ca7e..0054cb0 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -74,6 +74,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 	{ "instruction_sigp_restart", VCPU_STAT(instruction_sigp_restart) },
 	{ "diagnose_10", VCPU_STAT(diagnose_10) },
 	{ "diagnose_44", VCPU_STAT(diagnose_44) },
+	{ "diagnose_9c", VCPU_STAT(diagnose_9c) },
 	{ NULL }
 };
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 186ffab..2322b21 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -461,6 +461,7 @@ void mark_page_dirty_in_slot(struct kvm *kvm, struct kvm_memory_slot *memslot,
 
 void kvm_vcpu_block(struct kvm_vcpu *vcpu);
 void kvm_vcpu_kick(struct kvm_vcpu *vcpu);
+bool kvm_vcpu_yield_to(struct kvm_vcpu *target);
 void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu);
 void kvm_resched(struct kvm_vcpu *vcpu);
 void kvm_load_guest_fpu(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9eb7936..99a38a4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1543,6 +1543,31 @@ void kvm_resched(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_resched);
 
+bool kvm_vcpu_yield_to(struct kvm_vcpu *target)
+{
+	struct pid *pid;
+	struct task_struct *task = NULL;
+
+	rcu_read_lock();
+	pid = rcu_dereference(target->pid);
+	if (pid)
+		task = get_pid_task(target->pid, PIDTYPE_PID);
+	rcu_read_unlock();
+	if (!task)
+		return false;
+	if (task->flags & PF_VCPU) {
+		put_task_struct(task);
+		return false;
+	}
+	if (yield_to(task, 1)) {
+		put_task_struct(task);
+		return true;
+	}
+	put_task_struct(task);
+	return false;
+}
+EXPORT_SYMBOL_GPL(kvm_vcpu_yield_to);
+
 void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 {
 	struct kvm *kvm = me->kvm;
@@ -1561,8 +1586,6 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 	 */
 	for (pass = 0; pass < 2 && !yielded; pass++) {
 		kvm_for_each_vcpu(i, vcpu, kvm) {
-			struct task_struct *task = NULL;
-			struct pid *pid;
 			if (!pass && i < last_boosted_vcpu) {
 				i = last_boosted_vcpu;
 				continue;
@@ -1572,24 +1595,11 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
 				continue;
 			if (waitqueue_active(&vcpu->wq))
 				continue;
-			rcu_read_lock();
-			pid = rcu_dereference(vcpu->pid);
-			if (pid)
-				task = get_pid_task(vcpu->pid, PIDTYPE_PID);
-			rcu_read_unlock();
-			if (!task)
-				continue;
-			if (task->flags & PF_VCPU) {
-				put_task_struct(task);
-				continue;
-			}
-			if (yield_to(task, 1)) {
-				put_task_struct(task);
+			if (kvm_vcpu_yield_to(vcpu)) {
 				kvm->last_boosted_vcpu = i;
 				yielded = 1;
 				break;
 			}
-			put_task_struct(task);
 		}
 	}
 }
-- 
1.7.9.6

[PATCH 2/2] kvm-s390: use kvm_vcpu_on_spin for diag 0x44

[Christian Borntraeger]

Lets replace the old open coded version of diag 0x44 (which relied on
compat_sched_yield) with kvm_vcpu_on_spin.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/diag.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
index 2d2ae32..b23d9ac 100644
--- a/arch/s390/kvm/diag.c
+++ b/arch/s390/kvm/diag.c
@@ -47,9 +47,7 @@ static int __diag_time_slice_end(struct kvm_vcpu *vcpu)
 {
 	VCPU_EVENT(vcpu, 5, "%s", "diag time slice end");
 	vcpu->stat.diagnose_44++;
-	vcpu_put(vcpu);
-	yield();
-	vcpu_load(vcpu);
+	kvm_vcpu_on_spin(vcpu);
 	return 0;
 }
 
-- 
1.7.9.6

Re: [PATCH 1/2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Alexander Graf]

On 25.04.2012, at 15:30, 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.
> 
> Christian Borntraeger: moved common code in virt/kvm/
> 
> Signed-off-by: Konstantin Weitz <WEITZKON@de.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> arch/s390/include/asm/kvm_host.h |    1 +
> arch/s390/kvm/diag.c             |   25 +++++++++++++++++++++++
> arch/s390/kvm/kvm-s390.c         |    1 +
> include/linux/kvm_host.h         |    1 +
> virt/kvm/kvm_main.c              |   42 +++++++++++++++++++++++---------------
> 5 files changed, 54 insertions(+), 16 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index 7343872..dd17537 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -148,6 +148,7 @@ struct kvm_vcpu_stat {
> 	u32 instruction_sigp_restart;
> 	u32 diagnose_10;
> 	u32 diagnose_44;
> +	u32 diagnose_9c;
> };
> 
> struct kvm_s390_io_info {
> diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> index a353f0e..2d2ae32 100644
> --- a/arch/s390/kvm/diag.c
> +++ b/arch/s390/kvm/diag.c
> @@ -53,6 +53,29 @@ 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) {

Wouldn't

  kvm_get_vcpu(kvm, tid)

be what you want here?

Alex

Re: [PATCH 1/2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Christian Borntraeger]

>> +	kvm_for_each_vcpu(i, tcpu, kvm)
>> +		if (tcpu->vcpu_id == tid) {
> 
> Wouldn't
> 
>   kvm_get_vcpu(kvm, tid)
> 
> be what you want here?

Would be better in terms of scalability, but we can do that
only if kvm->vcpus[i].vcpu_id = i
Is that always the case?

Christian

Re: [PATCH 1/2] kvm-s390: Implement the directed yield (diag 9c) hypervisor call for KVM

[Alexander Graf]

On 26.04.2012, at 14:19, Christian Borntraeger wrote:

>>> +	kvm_for_each_vcpu(i, tcpu, kvm)
>>> +		if (tcpu->vcpu_id == tid) {
>> 
>> Wouldn't
>> 
>>  kvm_get_vcpu(kvm, tid)
>> 
>> be what you want here?
> 
> Would be better in terms of scalability, but we can do that
> only if kvm->vcpus[i].vcpu_id = i
> Is that always the case?

Ugh, unfortunately not. How about creating a tid -> vcpu map on init? Looping through all vcpus just to find the one with a static, linar id sounds a bit excessive to me :).


Alex

Re: [PATCH 0/2] rework kvm_vcpu_on_spin

[Avi Kivity]

On 04/25/2012 04:30 PM, Christian Borntraeger wrote:
> Avi,
>
> what about this approach?
>
>

Looks fine, and kvm_vcpu_on_spin() looks nicer as well.

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

Re: [PATCH 3/3] kvm-s390: implement KVM_CAP_MAX_VCPUS

[Marcelo Tosatti]

On Tue, Apr 24, 2012 at 04:01:40PM +0300, Avi Kivity wrote:
> On 04/24/2012 03:47 PM, Christian Borntraeger wrote:
> > > btw, a better place might be in kvm_devel_ioctl_check_extension_generic().
> >
> > Possibly yes. Dont know if there are architecture specific reasons to use
> > something different than KVM_MAX_VCPUS.
> 
> Okay, I'll fold the implementations unless something prevents it (unlikely).

KVM_CAP_NR_VCPUS = "recommended max_vcpus"
KVM_CAP_MAX_VCPUS = "maximum possible value for max_vcpus"

(commit 8c3ba334f8588e1d5)

CAP_NR_VCPUS is architecture dependent (depends on state of scability,
testing, etc).

Re: [PATCH 3/3] kvm-s390: implement KVM_CAP_MAX_VCPUS

[Marcelo Tosatti]

On Mon, Apr 30, 2012 at 09:51:47PM -0300, Marcelo Tosatti wrote:
> On Tue, Apr 24, 2012 at 04:01:40PM +0300, Avi Kivity wrote:
> > On 04/24/2012 03:47 PM, Christian Borntraeger wrote:
> > > > btw, a better place might be in kvm_devel_ioctl_check_extension_generic().
> > >
> > > Possibly yes. Dont know if there are architecture specific reasons to use
> > > something different than KVM_MAX_VCPUS.
> > 
> > Okay, I'll fold the implementations unless something prevents it (unlikely).
> 
> KVM_CAP_NR_VCPUS = "recommended max_vcpus"
> KVM_CAP_MAX_VCPUS = "maximum possible value for max_vcpus"
> 
> (commit 8c3ba334f8588e1d5)
> 
> CAP_NR_VCPUS is architecture dependent (depends on state of scability,
> testing, etc).

Both KVM_CAP_MAX_VCPUS and KVM_CAP_NR_VCPUS are architecture dependent.

Christian, you probably want to implement both (skipping this one
patch), applied remainder, thanks.

[PATCH 0/1] Updated Patch for NR/MAX VCPU on s390

[Christian Borntraeger]

Marcelo,

here is an updated patch. We will use 64 for max and supported numbers
of vcpus.

Christian Borntraeger (1):
  kvm-s390x: implement KVM_CAP_NR/MAX_VCPUS

 arch/s390/kvm/kvm-s390.c |    4 ++++
 1 file changed, 4 insertions(+)

-- 
1.7.9.6

[PATCH 1/1] kvm-s390x: implement KVM_CAP_NR/MAX_VCPUS

[Christian Borntraeger]

Let userspace know the number of max and supported cpus for kvm on s390.
Return KVM_MAX_VCPUS (currently 64) for both values.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 arch/s390/kvm/kvm-s390.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index d30c835..54f75ff 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -135,6 +135,10 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_SYNC_REGS:
 		r = 1;
 		break;
+	case KVM_CAP_NR_VCPUS:
+	case KVM_CAP_MAX_VCPUS:
+		r = KVM_MAX_VCPUS;
+		break;
 	default:
 		r = 0;
 	}
-- 
1.7.9.6

Re: [PATCH 1/1] kvm-s390x: implement KVM_CAP_NR/MAX_VCPUS

[Marcelo Tosatti]

On Wed, May 02, 2012 at 10:50:38AM +0200, Christian Borntraeger wrote:
> Let userspace know the number of max and supported cpus for kvm on s390.
> Return KVM_MAX_VCPUS (currently 64) for both values.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c |    4 ++++
>  1 file changed, 4 insertions(+)

Applied, thanks.

trace points and ABI

[Christian Borntraeger]

On 24/04/12 14:04, Avi Kivity wrote:
>> @@ -74,6 +74,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>>  	{ "instruction_sigp_restart", VCPU_STAT(instruction_sigp_restart) },
>>  	{ "diagnose_10", VCPU_STAT(diagnose_10) },
>>  	{ "diagnose_44", VCPU_STAT(diagnose_44) },
>> +	{ "diagnose_9c", VCPU_STAT(diagnose_9c) },
>>  	{ NULL }
>>  };
>>  
> 
> We're switching to tracepoints instead of homebrewed statistics.  It's
> in feature-removal-schedule.txt already.  It's okay to add them now, but
> please prepare for their removal.
> 

Avi,

Do you consider these tracepoints itself ABI or not?

we are preparing some patches that add trace points in addition to
a: these stats
b: our debug feature

If this is not ABI, then we could work on these trace points step by step,
e.g. first use the existing stat/debug as trace point places, then combine,
add or remove  trace points later on if necessary.

Christian

Re: trace points and ABI

[Avi Kivity]

On 06/06/2012 03:55 PM, Christian Borntraeger wrote:
> On 24/04/12 14:04, Avi Kivity wrote:
>>> @@ -74,6 +74,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>>>  	{ "instruction_sigp_restart", VCPU_STAT(instruction_sigp_restart) },
>>>  	{ "diagnose_10", VCPU_STAT(diagnose_10) },
>>>  	{ "diagnose_44", VCPU_STAT(diagnose_44) },
>>> +	{ "diagnose_9c", VCPU_STAT(diagnose_9c) },
>>>  	{ NULL }
>>>  };
>>>  
>> 
>> We're switching to tracepoints instead of homebrewed statistics.  It's
>> in feature-removal-schedule.txt already.  It's okay to add them now, but
>> please prepare for their removal.
>> 
> 
> Avi,
> 
> Do you consider these tracepoints itself ABI or not?


I'm not sure what the current kernel dogma is, but I would like to see a
clear separation of architectural tracepoints (=reporting information
defined by the hardware, from the guest point of view; for example an
instruction emulated or a hypercall called) and implementation
tracepoints that record an interesting event, but one that is dependent
on host-side kvm implementation.  Architectural tracepoints would be
soft ABIs (extension is allowed and in extreme cases changes), while
implementation tracepoints are completely fluid.  The current tools are
able to adapt to changes.

x86 kvm once had such a separation, with kvm: tracepoints being
architectural and kvmmmu: tracepoints being implementation defined.
Unfortunately there was some drift (vcpu_match_mmio does not correspond
to an architectural event).

> 
> we are preparing some patches that add trace points in addition to
> a: these stats
> b: our debug feature
> 
> If this is not ABI, then we could work on these trace points step by step,
> e.g. first use the existing stat/debug as trace point places, then combine,
> add or remove  trace points later on if necessary.
> 

I recommend doing the same split.  Architectural tracepoints are easy to
get right, and most useful to a developer.  Implementation tracepoints
are harder to get right, but the cost of getting them wrong is low.  In
any case I'd like to see the entire change in one patchset so we have
all the tracepoints in one kernel release cycle, not several.

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