Gregory Haskins wrote: > Gleb Natapov wrote: > >> On Mon, Jul 13, 2009 at 10:29:02AM -0400, Gregory Haskins wrote: >> >> >>> Gleb Natapov wrote: >>> >>> >>>> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c >>>> index 6c57e46..ce8fcd3 100644 >>>> --- a/virt/kvm/irq_comm.c >>>> +++ b/virt/kvm/irq_comm.c >>>> @@ -210,7 +210,8 @@ int kvm_request_irq_source_id(struct kvm *kvm) >>>> unsigned long *bitmap = &kvm->arch.irq_sources_bitmap; >>>> int irq_source_id; >>>> >>>> - mutex_lock(&kvm->irq_lock); >>>> + WARN_ON(!mutex_is_locked(&kvm->lock)); >>>> >>>> >>>> >>> Shouldn't this be fatal? (e.g. BUG_ON). I know the usage between >>> BUG/WARN is controversial, but it seems to me that something is >>> completely broken if you expect it to be locked and its not. Might as >>> well fail the system, IMO. >>> >>> >>> >> Well I don't really care but we have WARN_ON() in the code currently. >> >> > > Well, that is perhaps unfortunate, but not relevant. I am not reviewing > those patches ;) > > >> Besides the chances are good that even without locking around this >> function nothing will break, so why kill host kernel? >> >> > > The question to ask is: Is it legal to continue to run if the mutex is > found unlocked? If not, the offending caller should be found/fixed as > early as possible IMO, and an oops should be sufficient to do so. I > think WARN_ON tends to gets overused/abused, so lets not perpetuate it > simply because of precedence. > Err..precedent, I mean. Heh. /me needs more coffee. -Greg > Kind Regards, > -Greg > > >
Attachment:
signature.asc
Description: OpenPGP digital signature
