Re: [PATCH] netfilter: use per-cpu recursive spinlock (v6)

From: Linus Torvalds
Date: Thu Apr 16 2009 - 12:47:56 EST




On Thu, 16 Apr 2009, Eric Dumazet wrote:
>
> +struct rec_lock {
> + spinlock_t lock;
> + int count; /* recursion count */
> +};
> +static DEFINE_PER_CPU(struct rec_lock, arp_tables_lock);

What the _fuck_ are you doing?

Stop sending these shit-for-brains crazy patches out. That's not a lock,
that's a messy way of saying "I don't know what the hell I'm doing, but
I'll mess things up".

Don't do recursive locks (or your own locking primitives in general), but
goddammit, if you have to, at least know what the hell you're doing. Your
thing is a piece of shit.

A recursive lock needs an owner, or it's not a lock at all. It's some
random data structure that contains a lock that may or may not be taken,
and that may actually _work_ depending on the exact patterns of taking the
lock, but that's not an excuse.

The fact that code "happens to work by mistake" (and I'm not saying that
your does - but it might just because of the per-cpu'ness of it, and I'm
not even going to look at crap like that it closer to try to prove it one
way or the other) does not make that code acceptable.

Because even if it works today, it's just a bug waiting to happen. The
thing you did is _not_ a generic recursive lock, and it does _not_ work in
general. Don't call it a "rec_lock". Don't write code that accesses it
without any comments as if it was simple. Just DON'T.

Guys, this whole discussion has just been filled with crazy crap. Can
somebody even explain why we care so deeply about some counters for
something that we just _deleted_ and that have random values anyway?

I can see the counters being interesting while a firewall is active, but I
sure don't see what's so wonderfully interesting after-the-fact about a
counter on something that NO LONGER EXISTS that it has to be somehow
"exactly right".

And it's certainly not interesting enough to merit this kind of random
fragile crazy code.

Please. Get a grip, people!

Show of hands, here: tell me a single use that really _requires_ those
exact counters of a netfilter rule that got deleted and is no longer
active?

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/