On Tue, 19 March 2013 06:38:52 -0700, Greg Kroah-Hartman wrote: > On Mon, Mar 18, 2013 at 11:58:03PM -0400, Jörn Engel wrote: > > > > It compiles. I don't have a good testcase, so the procedure is to > > throw it into the test infrastructure and wait a week. > > Really? Please make a test cast to test it out properly. So you are willing to reject a patch that clearly fixes a bug because there is no testcase? And given that the bug is a race, any testcase will still boil down to "run this for as long as it took to reproduce the problem, then triple the time". I can reproduce the problem in our test infrastructure maybe twice a week, so I could call that a testcase just to provide you some formalistic argument to accept the patch. Of course the alternatives would be to stop using kref or to leave the bug in. I really wish I wouldn't have to contemplate either of those. > > The alternative would be to call local_irq_restore(flags); from > > kref_put_spinlock_irqsave() and not pass the flags. Getting rid of > > the extra parameter would be nice. But I'm not sure I want to prove > > that > > spin_unlock(lock); > > local_irq_restore(flags); > > is the same as > > spin_unlock_irqrestore(lock, flags); > > on all architectures and with all combinations of CONFIG options. > > It should be. We agree then. I will change the patch and see how it behaves in our setup. Jörn -- Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. -- Brian W. Kernighan -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
