Messages in this thread | | | Date | Tue, 16 Jun 2009 11:59:34 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [GIT]: Networking |
| |
On Tue, 16 Jun 2009, Eric Dumazet wrote: > > Here is first patch to take into account this initial offset in sk_wmem_alloc > > (Only compiled, not tested)
I think this is incredibly ugly and hacky.
> @@ -162,7 +162,7 @@ static void atalk_destroy_timer(unsigned long data) > { > struct sock *sk = (struct sock *)data; > > - if (atomic_read(&sk->sk_wmem_alloc) || > + if (atomic_read(&sk->sk_wmem_alloc) > 1 || > atomic_read(&sk->sk_rmem_alloc)) {
Seriously, look at that code, and tell me it makes sense.
No, it does not. The code looks like totally random line noise, and that whole "> 1" test makes no conceptual sense what-so-ever.
It _will_ result in random bugs later on, because code that doesn't make sense will never be good in the long run.
At the very least, add a helper function for "do I actually have outstanding allocations" or something like that. IOW, do a
/* * Comment here about that magical "1" */ static inline int sk_has_allocations(struct sock *sk) { return atomic_read(&sk->sk_wmem_alloc) > 1 || atomic_read(&sk->sk_rmem_alloc); }
and then make the various network protocols use that, rather than open-coding some random internal implementation magic.
Linus
| |