On Sat, 12 Jan 2008, Stephen Hemminger wrote:
> On Sat, 12 Jan 2008 11:40:10 +0200
> "Ilpo Järvinen" <[EMAIL PROTECTED]> wrote:
...snip...
> > built-in.o:
> > 12 functions changed, 250 bytes added, 1695 bytes removed, diff: -1445
...snip...
> > include/net/tcp.h | 35 +----------------------------------
> > net/ipv4/tcp.c | 35 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 36 insertions(+), 34 deletions(-)
> >
> > diff --git a/include/net/tcp.h b/include/net/tcp.h
> > index 48081ad..306580c 100644
> > --- a/include/net/tcp.h
> > +++ b/include/net/tcp.h
> > @@ -926,40 +926,7 @@ static const char *statename[]={
> > "Close Wait","Last ACK","Listen","Closing"
> > };
> > #endif
> > -
> > -static inline void tcp_set_state(struct sock *sk, int state)
> > -{
> > - int oldstate = sk->sk_state;
> > -
> > - switch (state) {
> > - case TCP_ESTABLISHED:
> > - if (oldstate != TCP_ESTABLISHED)
> > - TCP_INC_STATS(TCP_MIB_CURRESTAB);
> > - break;
> > -
> > - case TCP_CLOSE:
> > - if (oldstate == TCP_CLOSE_WAIT || oldstate == TCP_ESTABLISHED)
> > - TCP_INC_STATS(TCP_MIB_ESTABRESETS);
> > -
> > - sk->sk_prot->unhash(sk);
> > - if (inet_csk(sk)->icsk_bind_hash &&
> > - !(sk->sk_userlocks & SOCK_BINDPORT_LOCK))
> > - inet_put_port(&tcp_hashinfo, sk);
> > - /* fall through */
> > - default:
> > - if (oldstate==TCP_ESTABLISHED)
> > - TCP_DEC_STATS(TCP_MIB_CURRESTAB);
> > - }
> > -
> > - /* Change state AFTER socket is unhashed to avoid closed
> > - * socket sitting in hash tables.
> > - */
> > - sk->sk_state = state;
> > -
> > -#ifdef STATE_TRACE
> > - SOCK_DEBUG(sk, "TCP sk=%p, State %s -> %s\n",sk,
> > statename[oldstate],statename[state]);
> > -#endif
> > -}
> >
>
>
> Since the function is called with a constant state, I guess the assumption
> was that gcc would be smart enough to only include the code needed, it looks
> like
> either code was bigger or the compiler was dumber than expected
I'd guess that compiler was just dumber... :-) It might be an interesting
experiment to convert it to if's and see if it would make a difference,
maybe it just gets confused by the switch or something.
Besides, it not always that obvious that gcc is able to determine "the
constant state", considering e.g., the complexity in the cases with
tcp_rcv_synsent_state_process, tcp_close_state, tcp_done. In such cases
uninlining should be done and gcc is probably not able to mix both cases
nicely for a single function?
However, after looking a bit, I'm partially leaning towards the other
option too:
> > tcp_done | -145
> > tcp_disconnect | -141
...These called for tcp_set_state just _once_, while this calls for it
twice:
> > tcp_fin | -86
...Obviously the compiler was able to perform some reductions but 43 bytes
per inlining seems still a bit high number.
--
i.
