Re: [RFC] lapic3: remove the union of user/kernel irq in struct vcpu

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

He, Qing wrote:
> Avi,
> 	We have found a new bug about irqchip_in_kernel checking, as
> addressed by the following patch:
>
> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
> index 88b0b89..69518f8 100644
> --- a/drivers/kvm/kvm_main.c
> +++ b/drivers/kvm/kvm_main.c
> @@ -283,7 +283,8 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_init);
>  void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
>  {
>  	kvm_mmu_destroy(vcpu);
> -	kvm_free_apic(vcpu->apic);
> +	if (irqchip_in_kernel(vcpu->kvm))
> +		kvm_free_apic(vcpu->apic);
>  	free_page((unsigned long)vcpu->pio_data);
>  	free_page((unsigned long)vcpu->run);
>  }
>
> 	This is similar with what we try to fix several days ago. We
> could certainly fix them as they appear, however, I'm beginning to
> wonder, is the union of user/kernel irq really worthwhile? With this
> union, we may save some space, but there is cost. We have to do run-time
> checking which is time consuming, and there are potentially more bugs
> which will eventually bite us someday. If we remove it, making
> irq_summary and kernel apic not disturbing each other, the checking is
> saved, and we don't have to worry about future bugs caused by
> irq_summary, apic sharing any more.
>
>   

I'm not opposed to the removal of the union (the space savings are 
hardly significant).  But will it really remove a source of bugs or just 
hide them deeper?  You'll need the tests most of the time anyway.

I'll apply a union-removing patch if you send one.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/