lkml.org 
[lkml]   [2009]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2009-10-18 23:28    [W:0.097 / U:0.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site