[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090429050913.GA16683@localhost>
Date: Wed, 29 Apr 2009 13:09:13 +0800
From: Wu Fengguang <fengguang.wu@...el.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kosaki.motohiro@...fujitsu.com" <kosaki.motohiro@...fujitsu.com>,
"andi@...stfloor.org" <andi@...stfloor.org>,
"mpm@...enic.com" <mpm@...enic.com>,
"adobriyan@...il.com" <adobriyan@...il.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
Stephen Rothwell <sfr@...b.auug.org.au>,
Chandra Seetharaman <sekharan@...ibm.com>,
Nathan Lynch <ntl@...ox.com>, Olof Johansson <olof@...om.net>,
Helge Deller <deller@...isc-linux.org>
Subject: Re: [PATCH 5/5] proc: export more page flags in /proc/kpageflags
On Wed, Apr 29, 2009 at 11:48:29AM +0800, Wu Fengguang wrote:
> On Wed, Apr 29, 2009 at 10:55:27AM +0800, Andrew Morton wrote:
> > On Wed, 29 Apr 2009 10:38:42 +0800 Wu Fengguang <fengguang.wu@...el.com> wrote:
> >
> > > > > +#define kpf_copy_bit(uflags, kflags, visible, ubit, kbit) \
> > > > > + do { \
> > > > > + if (visible || genuine_linus()) \
> > > > > + uflags |= ((kflags >> kbit) & 1) << ubit; \
> > > > > + } while (0);
> > > >
> > > > Did this have to be implemented as a macro?
> > > >
> > > > It's bad, because it might or might not reference its argument, so if
> > > > someone passes it an expression-with-side-effects, the end result is
> > > > unpredictable. A C function is almost always preferable if possible.
> > >
> > > Just tried inline function, the code size is increased slightly:
> > >
> > > text data bss dec hex filename
> > > macro 1804 128 0 1932 78c fs/proc/page.o
> > > inline 1828 128 0 1956 7a4 fs/proc/page.o
> > >
> >
> > hm, I wonder why. Maybe it fixed a bug ;)
> >
> > The code is effectively doing
> >
> > if (expr1)
> > something();
> > if (expr1)
> > something_else();
> > if (expr1)
> > something_else2();
> >
> > etc. Obviously we _hope_ that the compiler turns that into
> >
> > if (expr1) {
> > something();
> > something_else();
> > something_else2();
> > }
> >
> > for us, but it would be good to check...
>
> By 'expr1', you mean (visible || genuine_linus())?
>
> No, I can confirm the inefficiency does not lie here.
>
> I simplified the kpf_copy_bit() to
>
> #define kpf_copy_bit(uflags, kflags, ubit, kbit) \
> uflags |= (((kflags) >> (kbit)) & 1) << (ubit);
>
> or
>
> static inline u64 kpf_copy_bit(u64 kflags, int ubit, int kbit)
> {
> return (((kflags) >> (kbit)) & 1) << (ubit);
> }
>
> and double checked the differences: the gap grows unexpectedly!
>
> text data bss dec hex filename
> macro 1829 168 0 1997 7cd fs/proc/page.o
> inline 1893 168 0 2061 80d fs/proc/page.o
> +3.5%
>
> (note: the larger absolute text size is due to some experimental code elsewhere.)
Wow, after simplifications the text size goes down by -13.2%:
text data bss dec hex filename
macro 1644 8 0 1652 674 fs/proc/page.o
inline 1644 8 0 1652 674 fs/proc/page.o
Amazingly we can now use inline function without performance penalty!
Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists