netdev
[Top] [All Lists]

Re: [RFC] cleaning up struct sock

To: "David S. Miller" <davem@xxxxxxxxxx>
Subject: Re: [RFC] cleaning up struct sock
From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxxxxxxxx>
Date: Tue, 11 Dec 2001 09:52:19 -0200
Cc: SteveW@xxxxxxx, jschlst@xxxxxxxxx, ncorbic@xxxxxxxxxxx, eis@xxxxxxxxxxxxx, dag@xxxxxxxxxxx, torvalds@xxxxxxxxxxxxx, marcelo@xxxxxxxxxxxxxxxx, netdev@xxxxxxxxxxx
In-reply-to: <20011210.231826.55509210.davem@xxxxxxxxxx>
References: <20011210230810.C896@xxxxxxxxxxxxxxxx> <20011210.231826.55509210.davem@xxxxxxxxxx>
Sender: owner-netdev@xxxxxxxxxxx
User-agent: Mutt/1.3.23i
Em Mon, Dec 10, 2001 at 11:18:26PM -0800, David S. Miller escreveu:
>    From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxxxxxxxx>
>    Date: Mon, 10 Dec 2001 23:08:10 -0200
>    
>    David, if you don't like it, I'll happily switch to the big fat
>    union idea, but I think that this is more clean and will avoid us
>    having to patch sock.h every time a new net stack is added to the
>    kernel.
> 
> I'm a little concerned about having to allocate two objects instead of
> one.

If that is the case then we could keep some of the performance critical
protocols in the union and leave the other ones, that already were
allocating two objects, using the sk->protinfo.generic (aka destruct_hook),
but there are other possibilities, like you mention.

Ok, the goal of not having anything specific to a protocol in sock.h would
not be achieved, but things more cleaner than today.

That was one of the reasons for me not to have left af_inet with the
#ifdefs in protinfo in this patch (i.e. for performance critical, most of
the time enabled anyway protocols, leave it as is in protinfo).

But lets see first if we find a clean way to have sock.h clean of protocol
specific stuff without harming performance.
 
> These things aren't like inodes.  Inodes are cached and lookup
> read-multiple objects, whereas sockets are throw-away and recycled
> objects.  Inode allocation performance therefore isn't that critical,
> but socket allocation performance is.
> 
> Then we go back to the old problem of protocols that can be used to
> embed IP and thus having to keep track of parallel state for multiple
> protocols.  I think your changes did not compromise that for what
> we currently support though.
> 
> I still need to think about this some more....
> 
> You know, actually, the protocols are the ones which call sk_alloc().
> So we could just extend sk_alloc() to take a kmem_cache_t argument.
> TCP could thus make a kmem_cache_t which is sizeof(struct sock)
> + sizeof(struct tcp_opt) and then set the TP_INFO pointer to "(sk +
> 1)".

yes, that is a nice idea, and if we have it like this:

struct sock {
        .
        .
        .
        void tp_pinfo[0];
}

#define TCP_PINFO(SK)   ((struct tcp_opt *)(&(SK)->tp_pinfo)
 
> Oh yes, another overhead is all the extra dereferencing.  To fight
> that we could make a macro that knows the above layout:
> 
> #define TCP_PINFO(SK) ((struct tcp_opt *)((SK) + 1))
> 
> So I guess we could do things your way without any of the potential
> performance problems.

yup, the patch I presented was just something quick so that more people
could talk think about it and to see if I should spend more time on it.

> It is going to be a while before I can apply something like this, I
> would like to help Jens+Linus get the new block stuff in shape first.
> This would obviously be a 2.5.x change too.

yes, of course, but I wanted to tell people that this was something I
wanted to have done as soon as possible, so that net stack maintainers
don't get too much patch clashes.

Well I'll think more about it, about what Steven told and try to come out
with something more polished.

- Arnaldo

<Prev in Thread] Current Thread [Next in Thread>