LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Rientjes <rientjes@google.com>
To: Joe Perches <joe@perches.com>
Cc: Matthew Wilcox <matthew@wil.cx>,
	LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, gcc@gcc.gnu.org
Subject: Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros
Date: Wed, 27 Feb 2008 14:58:27 -0800 (PST)	[thread overview]
Message-ID: <alpine.DEB.1.00.0802271454430.3836@chino.kir.corp.google.com> (raw)
In-Reply-To: <1204095249.19319.444.camel@localhost>

On Tue, 26 Feb 2008, Joe Perches wrote:

> > Joe, what version of gcc are you using?
> 
> $ gcc --version
> gcc (GCC) 4.2.2 20071128 (prerelease) (4.2.2-3.1mdv2008.0)
> 
> It's definitely odd.
> The .o size changes are inconsistent.
> Some get bigger, some get smaller.
> 
> The versioning ones I understand but I have no idea why
> changes in drivers/ or mm/ or net/ exist.
> 

When I did the same comparisons on my x86_64 defconfig with gcc 4.1.3, I 
only saw differences in drivers/ and fs/.

> I think it's gcc optimization changes, but dunno...
> Any good ideas?
> 

What's interesting about this is that it doesn't appear to be related to 
your change (static inline function to macro definition).  It appears to 
be simply removing the static inline function.

The only reference to __simple_attr_check_format() in either the x86 or 
x86_64 defconfig is via DEFINE_SIMPLE_ATTRIBUTE() in fs/debugfs/file.c.

If you remove the only reference to it:

diff --git a/include/linux/fs.h b/include/linux/fs.h
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2044,7 +2044,6 @@ static inline void simple_transaction_set(struct file *file, size_t n)
 #define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)		\
 static int __fops ## _open(struct inode *inode, struct file *file)	\
 {									\
-	__simple_attr_check_format(__fmt, 0ull);			\
 	return simple_attr_open(inode, file, __get, __set, __fmt);	\
 }									\
 static struct file_operations __fops = {				\

The text size remains the same:

   text    data     bss     dec     hex filename
5386111  846328  719560 6951999  6a143f vmlinux.before
5386111  846328  719560 6951999  6a143f vmlinux.after

Yet if you remove the reference _and_ the static inline function itself, 
replacing it with nothing:

diff --git a/include/linux/fs.h b/include/linux/fs.h
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2044,7 +2044,6 @@ static inline void simple_transaction_set(struct file *file, size_t n)
 #define DEFINE_SIMPLE_ATTRIBUTE(__fops, __get, __set, __fmt)		\
 static int __fops ## _open(struct inode *inode, struct file *file)	\
 {									\
-	__simple_attr_check_format(__fmt, 0ull);			\
 	return simple_attr_open(inode, file, __get, __set, __fmt);	\
 }									\
 static struct file_operations __fops = {				\
@@ -2055,12 +2054,6 @@ static struct file_operations __fops = {				\
 	.write	 = simple_attr_write,					\
 };
 
-static inline void __attribute__((format(printf, 1, 2)))
-__simple_attr_check_format(const char *fmt, ...)
-{
-	/* don't do anything, just let the compiler check the arguments; */
-}
-
 int simple_attr_open(struct inode *inode, struct file *file,
 		     int (*get)(void *, u64 *), int (*set)(void *, u64),
 		     const char *fmt);

The text size does become smaller:

   text    data     bss     dec     hex filename
5386111  846328  719560 6951999  6a143f vmlinux.before
5386047  846328  719560 6951935  6a13ff vmlinux.after

gcc 4.0.3 maintains the same text size for both cases, while it appears 
gcc 4.1.3 and your version, 4.2.2, have this different behavior.

		David

  parent reply	other threads:[~2008-02-27 22:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-27  3:08 [PATCH] linux/kernel.h linux/device.h " Joe Perches
2008-02-27  4:02 ` [PATCH] linux/fs.h " Joe Perches
2008-02-27  4:13   ` Matthew Wilcox
2008-02-27  4:55     ` Joe Perches
2008-02-27  5:44     ` David Rientjes
2008-02-27  6:54       ` Joe Perches
2008-02-27  7:38         ` David Rientjes
2008-02-27 22:58         ` David Rientjes [this message]
2008-02-27 23:58           ` Jan Hubicka
2008-02-28  8:28             ` David Rientjes
2008-02-28  8:42             ` Jakub Jelinek
2008-02-28 10:23               ` Jan Hubicka
2008-02-29  1:09                 ` Joe Perches
2008-03-23 15:22                   ` Denys Vlasenko
2008-03-24 19:52                     ` Joe Perches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.1.00.0802271454430.3836@chino.kir.corp.google.com \
    --to=rientjes@google.com \
    --cc=gcc@gcc.gnu.org \
    --cc=joe@perches.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH] linux/fs.h - Convert debug functions declared inline __attribute__((format (printf,x,y) to statement expression macros' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).