Messages in this thread | | | Date | Sat, 3 Mar 2012 15:01:25 +0530 | Subject | Re: [PATCH 1/3] qla3xxx: ethernet: Fix bogus interrupt state flag. | From | santosh prasad nayak <> |
| |
On Sat, Mar 3, 2012 at 2:43 AM, David Miller <davem@davemloft.net> wrote: > From: santosh prasad nayak <santoshprasadnayak@gmail.com> > Date: Fri, 2 Mar 2012 21:24:29 +0530 > >> In "ql_adapter_initialize", first unlock is done by >> "spin_unlock_irqrestore(&qdev->hw_lock, hw_flags)" >> with "hw_flags = 0" ("hw_flags" is local variable and initialized to >> zero.), which is as good as >> spin_unlock_irq. > > You must never pass to irqrestore anything other than a hw_flags > value given by irqsave or similar.
David,
Thats what my point is.
The function call is as follow:
ql_adapter_up() { ..... spin_lock_irqsave(&qdev->hw_lock, hw_flags); ..... err = ql_adapter_initialize(qdev); ..... spin_unlock_irqrestore(&qdev->hw_lock, hw_flags); ...... }
ql_adapter_initialize() { unsigned long hw_flags = 0; // D ....... spin_unlock_irqrestore(&qdev->hw_lock, hw_flags); // A msleep(500); // B spin_lock_irqsave(&qdev->hw_lock, hw_flags); // C ..... }
In ql_adapter_initialize, at A, 'spin_unlock_irqrestore' is called with "hw_flags = 0", which is as good as spin_unlock_irq().
Static analyzer is showing it as "Error : bogus hw_flags", which is true. Because "hw_flags" is initialized to zero at D and the same "hw_flags" is used to restore IRQ at A.
If intention of the developer is to unlock and enable IRQ at A then we can use "spin_unlock_irq()" which will remove the static analyzer error and also give better performance.
Regards Santosh
> You may not assume anything about what values hw_flags takes on nor > what those values might mean, they are architecture specific so > you may not just set it to zero and assume that does anything in > particular. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |