netdev - Re: [PATCH] netfilter: use per-cpu recursive lock (v11)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.00.0904220806320.3101@localhost.localdomain>
Date:	Wed, 22 Apr 2009 08:19:07 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Eric Dumazet <dada1@...mosbay.com>
cc:	Ingo Molnar <mingo@...e.hu>,
	Stephen Hemminger <shemminger@...tta.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>, paulmck@...ux.vnet.ibm.com,
	Evgeniy Polyakov <zbr@...emap.net>,
	David Miller <davem@...emloft.net>, kaber@...sh.net,
	jeff.chua.linux@...il.com, laijs@...fujitsu.com,
	jengelh@...ozas.de, r000n@...0n.net, linux-kernel@...r.kernel.org,
	netfilter-devel@...r.kernel.org, netdev@...r.kernel.org,
	benh@...nel.crashing.org, mathieu.desnoyers@...ymtl.ca
Subject: Re: [PATCH] netfilter: use per-cpu recursive lock (v11)

On Wed, 22 Apr 2009, Eric Dumazet wrote:
> 
> If this could be done without recursion, I am pretty sure netfilter
> and network guys would have done it. I found Linus reaction quite
> shocking IMHO, considering hard work done by all people on this.

You don't _understand_ do you?

There is a huge difference between recursive code, and a recursive lock.

The netfilter code may need to occasionally re-enter itself. Nobody ever 
contested _that_ part.

What I have disagreed with the whole time is 

 (a) doing local ad-hoc locking primitives without any comments 
     what-so-ever.

 (b) Doing them _wrong_ in many cases

 (c) Calling the _lock_ a "recursive" lock.

The fact that a lock works with recursion doesn't make it "recursive". 
That generally has a very special meaning for locking primitives, and 
means something else.

In contrast, a read-write lock actually has known properties, and we have 
existing locking mechanisms for those. And we call them read-write locks 
DESPITE THE FACT that the reading part can be done recursively. 

If you call a read-write lock a "recursive" lock, then you're a moron. 
It simply is _not_ a recursive lock. And neither is the lock you actually 
implemented, even though you (and Stephen) continually call it that. 

SO STOP CALLING IT A RECURSIVE LOCK. Look at your very own code: you can 
actually only use that lock in a recursive context in a _very_ specific 
place. Notice how it's only "recursive" when taken in the per-CPU context, 
but _not_ recursive when the filter-updating code ("writer") takes it?

Do you understand now? It really shouldn't be so hard for you. 

Naming is important. Locking is important. You did both things wrong. You 
named your locks something incorrect and mis-leading that didn't actually 
describe them, and you did your own private locking code without then 
documenting what the rules for this special lock were.

Maybe in your world that's ok. But no, in mine it's not. I've seen too 
many damn _non-functioning_ locks to ever want to see stuff like that 
again.

			Linus
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html