* KVM: x86: question about kvm_ioapic_destroy
@ 2015-04-26 17:19 Julia Lawall
2015-04-27 10:05 ` Michael S. Tsirkin
0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2015-04-26 17:19 UTC (permalink / raw)
To: zhanghy, mst, jasowang, zhanghy, pbonzini, x86, kvm, linux-kernel
The function kvm_ioapic_destroy is defined as follows:
void kvm_ioapic_destroy(struct kvm *kvm)
{
struct kvm_ioapic *ioapic = kvm->arch.vioapic;
cancel_delayed_work_sync(&ioapic->eoi_inject);
if (ioapic) {
kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, &ioapic->dev);
kvm->arch.vioapic = NULL;
kfree(ioapic);
}
}
Is there any way that cancel_delayed_work_sync can work if ioapic is NULL?
Should the call be moved down under the NULL test? Or is the NULL test
not needed? The NULL test has been there longer than the call to
cancel_delayed_work_sync, which was introduced in 184564ef.
thanks,
julia
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: KVM: x86: question about kvm_ioapic_destroy
2015-04-26 17:19 KVM: x86: question about kvm_ioapic_destroy Julia Lawall
@ 2015-04-27 10:05 ` Michael S. Tsirkin
2015-04-27 12:13 ` Paolo Bonzini
2015-04-27 12:32 ` Julia Lawall
0 siblings, 2 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2015-04-27 10:05 UTC (permalink / raw)
To: Julia Lawall; +Cc: zhanghy, jasowang, pbonzini, x86, kvm, linux-kernel
On Sun, Apr 26, 2015 at 07:19:58PM +0200, Julia Lawall wrote:
> The function kvm_ioapic_destroy is defined as follows:
>
> void kvm_ioapic_destroy(struct kvm *kvm)
> {
> struct kvm_ioapic *ioapic = kvm->arch.vioapic;
>
> cancel_delayed_work_sync(&ioapic->eoi_inject);
> if (ioapic) {
> kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, &ioapic->dev);
> kvm->arch.vioapic = NULL;
> kfree(ioapic);
> }
> }
>
> Is there any way that cancel_delayed_work_sync can work if ioapic is NULL?
> Should the call be moved down under the NULL test? Or is the NULL test
> not needed? The NULL test has been there longer than the call to
> cancel_delayed_work_sync, which was introduced in 184564ef.
>
> thanks,
> julia
I think the NULL test is not needed.
kvm_ioapic_destroy is only called if kvm_ioapic_init
completed successfully, and that sets kvm->arch.vioapic.
--
MST
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: KVM: x86: question about kvm_ioapic_destroy
2015-04-27 10:05 ` Michael S. Tsirkin
@ 2015-04-27 12:13 ` Paolo Bonzini
2015-04-27 12:32 ` Julia Lawall
1 sibling, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2015-04-27 12:13 UTC (permalink / raw)
To: Michael S. Tsirkin, Julia Lawall
Cc: zhanghy, jasowang, x86, kvm, linux-kernel
On 27/04/2015 12:05, Michael S. Tsirkin wrote:
> On Sun, Apr 26, 2015 at 07:19:58PM +0200, Julia Lawall wrote:
>> The function kvm_ioapic_destroy is defined as follows:
>>
>> void kvm_ioapic_destroy(struct kvm *kvm)
>> {
>> struct kvm_ioapic *ioapic = kvm->arch.vioapic;
>>
>> cancel_delayed_work_sync(&ioapic->eoi_inject);
>> if (ioapic) {
>> kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, &ioapic->dev);
>> kvm->arch.vioapic = NULL;
>> kfree(ioapic);
>> }
>> }
>>
>> Is there any way that cancel_delayed_work_sync can work if ioapic is NULL?
>> Should the call be moved down under the NULL test? Or is the NULL test
>> not needed? The NULL test has been there longer than the call to
>> cancel_delayed_work_sync, which was introduced in 184564ef.
>
> I think the NULL test is not needed.
> kvm_ioapic_destroy is only called if kvm_ioapic_init
> completed successfully, and that sets kvm->arch.vioapic.
Agreed. By the way, in that case the cancel_delayed_work_sync is really
a nop.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: KVM: x86: question about kvm_ioapic_destroy
2015-04-27 10:05 ` Michael S. Tsirkin
2015-04-27 12:13 ` Paolo Bonzini
@ 2015-04-27 12:32 ` Julia Lawall
1 sibling, 0 replies; 4+ messages in thread
From: Julia Lawall @ 2015-04-27 12:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Julia Lawall, zhanghy, jasowang, pbonzini, x86, kvm, linux-kernel
On Mon, 27 Apr 2015, Michael S. Tsirkin wrote:
> On Sun, Apr 26, 2015 at 07:19:58PM +0200, Julia Lawall wrote:
> > The function kvm_ioapic_destroy is defined as follows:
> >
> > void kvm_ioapic_destroy(struct kvm *kvm)
> > {
> > struct kvm_ioapic *ioapic = kvm->arch.vioapic;
> >
> > cancel_delayed_work_sync(&ioapic->eoi_inject);
> > if (ioapic) {
> > kvm_io_bus_unregister_dev(kvm, KVM_MMIO_BUS, &ioapic->dev);
> > kvm->arch.vioapic = NULL;
> > kfree(ioapic);
> > }
> > }
> >
> > Is there any way that cancel_delayed_work_sync can work if ioapic is NULL?
> > Should the call be moved down under the NULL test? Or is the NULL test
> > not needed? The NULL test has been there longer than the call to
> > cancel_delayed_work_sync, which was introduced in 184564ef.
> >
> > thanks,
> > julia
>
> I think the NULL test is not needed.
> kvm_ioapic_destroy is only called if kvm_ioapic_init
> completed successfully, and that sets kvm->arch.vioapic.
Thanks. I will send a patch.
julia
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-04-27 12:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-26 17:19 KVM: x86: question about kvm_ioapic_destroy Julia Lawall
2015-04-27 10:05 ` Michael S. Tsirkin
2015-04-27 12:13 ` Paolo Bonzini
2015-04-27 12:32 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox