On Fri, Oct 26, 2012 at 03:42:44PM +0200, Stefani Seibold wrote: > Am Freitag, den 26.10.2012, 21:04 +0800 schrieb Yuanhan Liu: > > On Fri, Oct 26, 2012 at 11:26:31AM +0200, Stefani Seibold wrote: > > > Am Freitag, den 26.10.2012, 15:17 +0800 schrieb Yuanhan Liu: > > > > On Fri, Oct 26, 2012 at 08:51:06AM +0200, Stefani Seibold wrote: > > > > > Am Freitag, den 26.10.2012, 14:11 +0800 schrieb Yuanhan Liu: > > > > > > On Fri, Oct 26, 2012 at 07:38:31AM +0200, Stefani Seibold wrote: > > > > > > > Am Freitag, den 26.10.2012, 09:46 +0800 schrieb Yuanhan Liu: > > > > > > > > From: Yuanhan Liu <yuanhan....@linux.intel.com> > > > > > > > > > > > > > > > > Firstly, this kind of type check doesn't work. It does > > > > > > > > something similay > > > > > > > > like following: > > > > > > > > void * __dummy = NULL; > > > > > > > > __buf = __dummy; > > > > > > > > > > > > > > > > __dummy is defined as void *. Thus it will not trigger warnings > > > > > > > > as > > > > > > > > expected. > > > > > > > > > > > > > > > > Second, we don't need that kind of check. Since the prototype > > > > > > > > of __kfifo_out is: > > > > > > > > unsigned int __kfifo_out(struct __kfifo *fifo, void > > > > > > > > *buf, unsigned int len) > > > > > > > > > > > > > > > > buf is defined as void *, so we don't need do the type check. > > > > > > > > Remove it. > > > > > > > > > > > > > > > > LINK: https://lkml.org/lkml/2012/10/25/386 > > > > > > > > LINK: https://lkml.org/lkml/2012/10/25/584 > > > > > > > > > > > > > > > > Cc: Andrew Morton <a...@linux-foundation.org> > > > > > > > > Cc: Wei Yang <weiy...@linux.vnet.ibm.com> > > > > > > > > Cc: Stefani Seibold <stef...@seibold.net> > > > > > > > > Cc: Fengguang Wu <fengguang...@intel.com> > > > > > > > > Cc: Stephen Rothwell <s...@canb.auug.org.au> > > > > > > > > Signed-off-by: Yuanhan Liu <yuanhan....@linux.intel.com> > > > > > > > > --- > > > > > > > > > [snip]... > > > > > > > > > > > > Also you have to build the kfifo samples, since this example code use > > > > > all features of the kfifo API. > > > > > > > > > > And again: The kfifo is designed to do the many things at compile > > > > > time, > > > > > not at runtime. If you modify the code, you have to check the compiler > > > > > assembler output for no degradation, especially in kfifo_put, > > > > > kfifo_get, > > > > > kfifo_in, kfifo_out, __kfifo_in and __kfifo_out. Prevent runtime > > > > > checks > > > > > if you can do it at compile time. This is the basic reasons to do it > > > > > in > > > > > macros. > > > > > > > > Is it enought to check kernel/kfifo.o only? I build that file with > > > > and without this patch. And then dump it by objdump -D kernel/fifo.o to > > > > /tmp/kfifo.dump.with and /tmp/kfifo.dump.without, respectively. And the > > > > two dump file are exactly same. > > > > > > > > > > No, since most of the code is inlined due performace reasons, you have > > > to hack the kfifo examples output code for regressions and code > > > increase. > > > > In my test, this patch doesn't change anything. Here are some data to > > prove that: > > > > $ make samples/kfifo/ > > $ cp samples/kfifo/*.o /tmp/before/ > > > > $ git am this-patch > > $ make samples/kfifo/ > > $ cp samples/kfifo/*.o /tmp/after/ > > > > $ for i in /tmp/before/*.o; do size $i /tmp/after/`basename $i`; done > > text data bss dec hex filename > > 1939 464 456 2859 b2b /tmp/before/bytestream-example.o > > 1939 464 456 2859 b2b /tmp/after/bytestream-example.o > > text data bss dec hex filename > > 1423 112 296 1831 727 /tmp/before/dma-example.o > > 1423 112 296 1831 727 /tmp/after/dma-example.o > > text data bss dec hex filename > > 1864 624 376 2864 b30 /tmp/before/inttype-example.o > > 1864 624 376 2864 b30 /tmp/after/inttype-example.o > > text data bss dec hex filename > > 1916 464 472 2852 b24 /tmp/before/record-example.o > > 1916 464 472 2852 b24 /tmp/after/record-example.o > > # You will see that it changed nothing. > > > > > > $ objdump -d /tmp/before/bytestream-example.o > > >/tmp/bytestream-example.before > > $ objdump -d /tmp/after/bytestream-example.o >/tmp/bytestream-example.after > > $ diff /tmp/bytestream.before /tmp/bytestream.after -urN > > --- bytestream.before 2012-10-26 20:55:33.645578668 +0800 > > +++ bytestream.after 2012-10-26 20:55:26.520578669 +0800 > > @@ -1,5 +1,5 @@ > > > > -/tmp/bytestream-example.o: file format elf64-x86-64 > > +/tmp/bytestream-example.o: file format elf64-x86-64 > > > > # So, as you can see, expect the filename, they are same. > > > > > > So, Stefani, is it what you want? Does this looks OK to you? > > Perfect. It looks okay for me and i hope for you too ;-) > > Acked by stef...@seibold.net
Thanks for that. Well, I checked this code a bit more, and found I forgot to remove something: ptr and const_ptr, which were used for type checking only. Since we are gonna remove type checking, we don't need them, too. I did same allmodconfig build, size, objdump test to all kfifo users for v2 patch as well, and found no new warning, error or assembler changes. Will sent out v2 soon. And sorry for not noticing this issue early. :( Thanks, Yuanhan Liu -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
