Re: [PATCH 2/2] module: fix bne2 "gave up waiting for init of modulelibcrc32c"

[Linus Torvalds]

On Tue, 1 Jun 2010, Rusty Russell wrote:
> 
> Sure, that simplicity has been eroded, but "crap" is harsh.

Crap is crap. Crap in locking is _especially_ dangerous.

> > module loading may well "work", but who the hell knows what it really 
> > results in?
> 
> I do.  If I didn't think so, I wouldn't have pushed the patch.

I'm sorry, but that's simply not good enough. We do not do ad-hoc locking 
"just becuse it works". Locking is too damn easy to get wrong, and "one 
person knows how the locking works" is not an excuse for anything at all.

> See, this I agree with, but you could have said this in far fewer words and
> much more politely.

Why? Why does "locking is crap" need any politeness? And why is it wrong 
to explain at length exactly _why_ our module locking is a piece of sh*t?

So explain why I should be more polite, or more terse?

> As posted, I had a patch to clean up the locking.  Seems you ignored it.

Umm. That patch was not the original one that I objected to, is it?

I do agree with your 1/2, btw, the one you posted under protest after I 
pointed out that the locking was crap. I was just explaining _why_ the 
locking was crap, and what the problem was to Andrew. 

I happen to also think that my solution to the problem is actually better 
and more straightforward than your one is. 

> > It's entirely possible that an interim fix (if we can't just fix the 
> > locking) is to _not_ use "strong_try_module_get()" at all, but instead 
> > just use "try_module_get()", and then after we've dropped the 
> > module_mutex, but _before_ we call the "init" function for the module, we 
> > wait for all the modules that this module depends on.
> 
> No, those modules could still fail init.

Umm. Which I took care of.

> > Doesn't that sound like the logical thing to do? And it wouldn't change 
> > any locking.
> 
> No, it sounds wrong, complex and fundamentally broken.

An dby "complex and fundamentally broken" you obviously mean "simpler and 
cleaner than the series posted by yourself". Or what?

I posted the damn series. Your next email even claims that my first one in 
the series (the bigger one) was a nice cleanup. You didn't comment on the 
second one, apparently because you're too embarrassed to admit you were 
wrong, and it wasn't all that 'wrong, complex, and fundamentally broken' 
to begin with. 

It was pretty damn straightforward, with no subtleties at all, in fact. 
Wasn't it?

		Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/