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

Linus Torvalds
2009-04-22T15:31:56Z

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