linux-kernel - 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]

Date:	Tue, 21 Apr 2009 13:14:56 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
cc:	Stephen Hemminger <shemminger@...tta.com>,
	Paul Mackerras <paulus@...ba.org>,
	Eric Dumazet <dada1@...mosbay.com>,
	Evgeniy Polyakov <zbr@...emap.net>,
	David Miller <davem@...emloft.net>, kaber@...sh.net,
	jeff.chua.linux@...il.com, mingo@...e.hu, 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 Tue, 21 Apr 2009, Paul E. McKenney wrote:
> 
> I believe that at least some of this is naming...

Maybe.

I do agree that the way netfilter would use the lock is somewhat different 
from a normal 'reader-writer' lock, since this special case of readers is 
about a per-cpu thing.

> So, would it help if the function names names in this patch said something
> about "local" and "global" rather than "read" and "write"?

Oh, I would have no problem at all with 'local' and 'global', in fact it 
would explain _why_ that read-write lock works.

The problem with naming I have is with the 'recursive' part. There is no 
ambiguity what-so-ever about what a "recursive lock" is (at least of the 
traditional kind), and the lock described here is not it.

So don't get me wrong - I could certainly live with a special lock in the 
networking. BUT:

 - it had damn well be documented as to what it does, and why it works

 - and it had better actually _work_, and not be buggy.

I suspect that using our regular reader-writer locks works well enough, 
and yes, it's probably worth making it really clear that the reader 
variety can only ever be used in the "local" form. That kind of is implied 
by the whole function, though.

And if somebody wants to open-code it as a per-cpu spinlock and a per-cpu 
'local counter', I can live with that too, but at that point I want people 
to just be a lot more careful, and also document it a lot more. 

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