[PATCH-2.6.0-tiny] "uninline" {lock,release}_sock

[PATCH-2.6.0-tiny] "uninline" {lock,release}_sock

[Arnaldo Carvalho de Melo]

Hi Matt,

	Please apply on top of your 2.6.0-tiny1 tree, CC to netdev for
eventual comments.

	With this patch we save more 3.321 bytes. At a first compilation
on top of plain 2.6.0 I thought I had seen it save more, will perhaps check
this later... if you optimized some of the kernel primitives that are called
by the current lock_sock/release_sock... Nah, too sleepy now :)

	Will test too with lock_sock/release_sock as an inline (or not if
CONFIG_NET_SMALL is selected) and not as a macro as it is today, does anybody
knows why it isn't already inline? Dave? /me must be missing something here...

   text    data     bss     dec     hex filename  patch        savings
 858052  163460   65068 1086580  109474 vmlinux   2.6.0-tiny1
 854697  163494   65068 1083259  10877b vmlinux   lock_sock       3321

Best Regards,

- Arnaldo

diff -urp linux-2.6.0.tiny.orig/include/net/sock.h linux-2.6.0.tiny.acme/include/net/sock.h
--- linux-2.6.0.tiny.orig/include/net/sock.h	2003-12-18 00:59:15.000000000 -0200
+++ linux-2.6.0.tiny.acme/include/net/sock.h	2003-12-28 01:32:50.000000000 -0200
@@ -542,7 +542,7 @@ static inline struct inode *SOCK_INODE(s
 extern void __lock_sock(struct sock *sk);
 extern void __release_sock(struct sock *sk);
 #define sock_owned_by_user(sk)	((sk)->sk_lock.owner)
-#define lock_sock(__sk) \
+#define _lock_sock(__sk) \
 do {	might_sleep(); \
 	spin_lock_bh(&((__sk)->sk_lock.slock)); \
 	if ((__sk)->sk_lock.owner) \
@@ -551,7 +551,7 @@ do {	might_sleep(); \
 	spin_unlock_bh(&((__sk)->sk_lock.slock)); \
 } while(0)
 
-#define release_sock(__sk) \
+#define _release_sock(__sk) \
 do {	spin_lock_bh(&((__sk)->sk_lock.slock)); \
 	if ((__sk)->sk_backlog.tail) \
 		__release_sock(__sk); \
@@ -561,6 +561,14 @@ do {	spin_lock_bh(&((__sk)->sk_lock.sloc
 	spin_unlock_bh(&((__sk)->sk_lock.slock)); \
 } while(0)
 
+#ifdef CONFIG_NET_SMALL
+extern void lock_sock(struct sock *sk);
+extern void release_sock(struct sock *sk);
+#else
+#define lock_sock(__sk) _lock_sock(__sk)
+#define release_sock(__sk) _release_sock(__sk)
+#endif
+
 /* BH context may only use the following locking interface. */
 #define bh_lock_sock(__sk)	spin_lock(&((__sk)->sk_lock.slock))
 #define bh_unlock_sock(__sk)	spin_unlock(&((__sk)->sk_lock.slock))
diff -urp linux-2.6.0.tiny.orig/net/core/sock.c linux-2.6.0.tiny.acme/net/core/sock.c
--- linux-2.6.0.tiny.orig/net/core/sock.c	2003-12-18 00:59:15.000000000 -0200
+++ linux-2.6.0.tiny.acme/net/core/sock.c	2003-12-28 01:32:50.000000000 -0200
@@ -1119,6 +1119,22 @@ void sock_init_data(struct socket *sock,
 	atomic_set(&sk->sk_refcnt, 1);
 }
 
+#ifdef CONFIG_NET_SMALL
+void lock_sock(struct sock *sk)
+{
+	_lock_sock(sk);
+}
+
+EXPORT_SYMBOL(lock_sock);
+
+void release_sock(struct sock *sk)
+{
+	_release_sock(sk);
+}
+
+EXPORT_SYMBOL(release_sock);
+#endif
+
 EXPORT_SYMBOL(__lock_sock);
 EXPORT_SYMBOL(__release_sock);
 EXPORT_SYMBOL(sk_alloc);

Re: [PATCH-2.6.0-tiny] "uninline" {lock,release}_sock

[Linus Torvalds]

On Sun, 28 Dec 2003, Arnaldo Carvalho de Melo wrote:
> 
> 	Please apply on top of your 2.6.0-tiny1 tree, CC to netdev for
> eventual comments.

Please don't do it this way.

This is quite possibly faster even _normally_, so it might make more sense 
to just do it globally instead of having a CONFIG_NEX_SMALL.

Basically, inline functions tend to win only when inlining them is smaller
and simpler than actually calling a function. The most common cause of
that is that some argument is commonly constant (and thus gets simplified
away by inlining), or the function itself literally expands to just a few 
instructions (the list functions, the inline asms for things like "cli" 
etc).

We use a lot too many inline functions for other reasons: one reason to 
use them is that they are sometimes more convenient than it is to find a 
good place for the non-inline version. Another common reason is that the 
thing started out smaller than it eventually became - and the inline just 
stuck around.

But if you do things like this for a CONFIG_SMALL, then the convenience 
argument obviously isn't true any more, and you'd be a lot better off just 
unconditionally making it a real function call.

Function calls aren't all that expensive, especially with FASTCALL() etc 
to show that you don't have to follow the common calling conventions. 
Right now I think FASTCALL() only matters on x86, but some other 
architectures could make it mean "smaller call clobbered list" or similar.

Have you benchmarked with the smaller kernel? 

		Linus

Re: [PATCH-2.6.0-tiny] "uninline" {lock,release}_sock

[David S. Miller]

On Sun, 28 Dec 2003 00:23:07 -0800 (PST)
Linus Torvalds <torvalds@osdl.org> wrote:

> Function calls aren't all that expensive, especially with FASTCALL() etc 
> to show that you don't have to follow the common calling conventions. 
> Right now I think FASTCALL() only matters on x86, but some other 
> architectures could make it mean "smaller call clobbered list" or similar.
> 
> Have you benchmarked with the smaller kernel? 

To be honest I think {lock,release}_sock() should both be uninlined
always.

It almost made sense to inline these things before the might_sleep()
was added, now it definitely makes no sense.

Re: [PATCH-2.6.0-tiny] "uninline" {lock,release}_sock

[Matt Mackall]

On Sun, Dec 28, 2003 at 01:23:29AM -0800, David S. Miller wrote:
> On Sun, 28 Dec 2003 00:23:07 -0800 (PST)
> Linus Torvalds <torvalds@osdl.org> wrote:
> 
> > Function calls aren't all that expensive, especially with FASTCALL() etc 
> > to show that you don't have to follow the common calling conventions. 
> > Right now I think FASTCALL() only matters on x86, but some other 
> > architectures could make it mean "smaller call clobbered list" or similar.
> > 
> > Have you benchmarked with the smaller kernel? 

The primary benchmark for -tiny is how much space it frees up, which
is very straightforward. More generally, I think we've got something
of a crisis on our hands in terms of benchmarking as caching
architectures are making microbenchmark results worse than worthless
for real life and changes like these are often lost in the noise for
larger benchmarks.
 
> To be honest I think {lock,release}_sock() should both be uninlined
> always.
> 
> It almost made sense to inline these things before the might_sleep()
> was added, now it definitely makes no sense.

For the purposes of my -tiny tree, I'd like to make every new feature
conditional as an aid to footprint measurement, benchmarking, regression
testing, etc. When I start feeding these patches to mainline, they can
be made unconditional as is warranted.

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting

Happy Birthday Linus !!!

[mjt]

if rumor has it correct, today is Linus' birthday.

... happy 34th Linus !!!


-- 
<<   http://michaeljtobler.homelinux.com/   >>
<<          author: "Inside Linux"          >>

There's no trick to being a humorist when you have the 
whole government working for you. - Will Rodgers

Re: Happy Birthday Linus !!!

[Johan Sjoholm]

And so it is! I'm joining in to the celebration!

> if rumor has it correct, today is Linus' birthday.
> ... happy 34th Linus !!!

-J

Re: Happy Birthday Linus !!!

[Matt Richards]

Happy Birthday :)

Johan Sjoholm wrote:

>And so it is! I'm joining in to the celebration!
>
>  
>
>>if rumor has it correct, today is Linus' birthday.
>>... happy 34th Linus !!!
>>    
>>
>
>-J
>
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at  http://www.tux.org/lkml/
>
>  
>

Re: Happy Birthday Linus !!!

[Thomas Zehetbauer]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

Thanks for the hint mjt, although I don't like clobbering the mailing
list I want to join the celebration and send my congratulations and best
wishes to Linus.

Tom

-- 
  T h o m a s   Z e h e t b a u e r   ( TZ251 )
  PGP encrypted mail preferred - KeyID 96FFCB89
       mail pgp-key-request@hostmaster.org

The horizon of many people is a circle with a radius of zero.
They call this their point of view.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 481 bytes --]

Re: Happy Birthday Linus !!!

[Jay Yost]

Happy birthday Linus!

Jay
										"Linux is like sex - it's better when it's free" - Linus Torvalds

"The trouble with computers of course is that they are very sophisticated 
idiots" - Tom Baker

Re: Happy Birthday Linus !!!

[Andre Hedrick]

Great where is the stinking paddle?
34 wacks would give great meaning to this year.

Whoot!

Andre Hedrick
LAD Storage Consulting Group

On Sun, 28 Dec 2003, mjt wrote:

> if rumor has it correct, today is Linus' birthday.
> 
> ... happy 34th Linus !!!
> 
> 
> -- 
> <<   http://michaeljtobler.homelinux.com/   >>
> <<          author: "Inside Linux"          >>
> 
> There's no trick to being a humorist when you have the 
> whole government working for you. - Will Rodgers
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>