Re: [PATCH] netfilter: use per-CPU r**ursive lock {XV}

[Stephen Hemminger <shemminger@vyatta.com>]

On Mon, 27 Apr 2009 13:58:48 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> 
> On Tue, 28 Apr 2009, Evgeniy Polyakov wrote:
> > 
> > Just ot be sure readers will not lose the discssion topic: do you object
> > against naming or realizaion?
> 
> I said _long_ ago that I thought the patch was fine. 
> 
> What I object to is people mis-representing the lock, and apparently 
> having a really hard time admitting that having a good grounding in 
> networking doesn't necessarily mean that you know everything about 
> something as basic as locking.
> 
> > If its about the former, does 'dog's breath lock' proposed by Stephen 
> > fit?
> 
> Is that just an attempt to avoid admitting that they were wrong about lock 
> naming? And then trying to trivialize it by calling things by a 
> _different_ wrong name, but silly enough that they hope people won't call 
> them on it?

The part that concerns me is that the reader lock used in a nested manner on
same cpu may still be broken on -rt. Other than that it is just language
lawyering; violent agreement that the lock gets used multiple times by
same CPU. I never had the occasion to address this before
(and avoided such usage), but this legacy code exists and needs to
be dealt with.


> Why not just use the correct name? I think it was Mathieu that just 
> suggested:
> 
> 	[PATCH] netfilter: use bh disabling with per-cpu read-write lock
> 
> or just call it "netfilter: use per-CPU read-write lock".

[PATCH] netfilter: Ceci n'est pas une serrure récurrente

I don't care. I don't care. Don't you get the point yet.



> 
> Why are people so against calling things by their correct names? Why do 
> certain network people seem to force a stupid and incorrect description, 
> when they have been told (a) that it's wrong and (b) why it's wrong 
> several times?

Because meaning comes from context, and my meaning comes from different
context. So we disagree on correct names. 

> What's so hard with just doing the TechnicallyRightThing(tm) and 
> describing it as such?
> 
> And btw - don't get me wrong. There are _other_ ways to do that locking 
> too. You don't have to use a rwlock. You can do it with explicit counting, 
> the way Eric's original patch did. But it would be wrong to call that one 
> "recursive lock" too.
> 
> Or you _could_ use a recursive lock, but nobody has actually posted such 
> patches. It would work. No question about it. And if it actually _were_ a 
> recursive lock, I wouldn't have objected about the description saying it 
> is (although I would probably have objected to it being unnecessarily 
> complex, when a simpler rwlock or the explicit count thing would be 
> sufficient).
> 
> But since the current simple patch is using a rwlock, why not just say 
> that? Why call it something incorrect ("recursive lock") or nonsensical 
> ("dog's breath lock").
> 
> As I tried to explain with an analogy, networking people would (quite 
> correctly) object to me calling a serial line an "ethernet cable". Why is 
> it so hard for netfilter people to then see why it's wrong to call a 
> rwlock a "recursive lock".
> 
> I mean, guys, if you don't want to read up on decades of locking work, 
> just google for it. Google for "recursive lock" (_with_ the quotes). At 
> least for me, the very first hit gives a reasonable explanation for it, 
> and it says:
> 
>   "POSIX allows mutexes to be recursive. That means the same thread can 
>    lock the same mutex twice and won't deadlock."
> 
> and realize that the "same thread" part is very much a keyword, not juat 
> a random implementation detail (the first answer to the question is 
> better than the question, but even the question at the top really does 
> get at the basics).
> 
> And please do realize that neither rwlocks nor the counting locks from 
> Dumazet's original patch do that. Never did. They simply aren't recursive 
> locks.
> 
> So just don't call them that. But is "dog's breath" any better? Yes, maybe 
> it's less _misleading_, but it sure as hell isn't any more descriptive.
> 
> 			Linus
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html