Re: [PATCH] direct-io: Deinline dio_zero_block, save 2684 bytes

Re: [PATCH] direct-io: Deinline dio_zero_block, save 2684 bytes

[Denys Vlasenko]

On Sun, Mar 13, 2016 at 10:00 PM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
> This function compiles to 2684 bytes, 2 callsites
>
>    text    data     bss     dec     hex filename
>    9655      16       0    9671    25c7 direct-io.o.before2
>    9559      16       0    9575    2567 direct-io.o

Al, you undoubtedly noticed the discrepancy between
"save 2684 bytes" claim in the commit description
and the above data, which shows no such thing.

Sorry, I was too quick to send the mail to notice it :(

Further investigation had shown that it's my particular version of gcc
deciding to deinline the function even before the patch.
Other version of gcc, on a different machine was running
large inlining search script, did not do that, and saw the 2684 bytes
of savings.

tl;dr: the patch is correct, my "size" printout wasn't.

Re: [PATCH] direct-io: Deinline dio_zero_block, save 2684 bytes

[Ming Lei]

On Mon, Mar 14, 2016 at 5:36 AM, Denys Vlasenko
<vda.linux@googlemail.com> wrote:
> On Sun, Mar 13, 2016 at 10:00 PM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
>> This function compiles to 2684 bytes, 2 callsites
>>
>>    text    data     bss     dec     hex filename
>>    9655      16       0    9671    25c7 direct-io.o.before2
>>    9559      16       0    9575    2567 direct-io.o
>
> Al, you undoubtedly noticed the discrepancy between
> "save 2684 bytes" claim in the commit description
> and the above data, which shows no such thing.
>
> Sorry, I was too quick to send the mail to notice it :(
>
> Further investigation had shown that it's my particular version of gcc
> deciding to deinline the function even before the patch.
> Other version of gcc, on a different machine was running
> large inlining search script, did not do that, and saw the 2684 bytes
> of savings.
>
> tl;dr: the patch is correct, my "size" printout wasn't.

Not sure the patch is correct, please see the commit for
do_blockdev_direct_IO():

 * NOTE: if you pass "sdio" to anything by pointer make sure that function
 * is always inlined. Otherwise gcc is unable to split the structure into
 * individual fields and will generate much worse code. This is important
 * for the whole file.



thanks,
Ming Lei

Re: [PATCH] direct-io: Deinline dio_zero_block, save 2684 bytes

[Jeff Moyer]

Denys Vlasenko <dvlasenk@redhat.com> writes:

> This function compiles to 2684 bytes, 2 callsites
>
>    text	   data	    bss	    dec	    hex	filename
>    9655	     16	      0	   9671	   25c7	direct-io.o.before2
>    9559	     16	      0	   9575	   2567	direct-io.o

NAK

[PATCH] direct-io: Deinline dio_zero_block, save 2684 bytes

[Denys Vlasenko]

This function compiles to 2684 bytes, 2 callsites

   text	   data	    bss	    dec	    hex	filename
   9655	     16	      0	   9671	   25c7	direct-io.o.before2
   9559	     16	      0	   9575	   2567	direct-io.o

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Al Viro <viro@zeniv.linux.org.uk>
CC: Jens Axboe <axboe@fb.com>
CC: Christoph Hellwig <hch@lst.de>
CC: linux-kernel@vger.kernel.org
---
 fs/direct-io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 21ec685..729c902 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -857,7 +857,7 @@ static void clean_blockdev_aliases(struct dio *dio, struct buffer_head *map_bh)
  * `end' is zero if we're doing the start of the IO, 1 at the end of the
  * IO.
  */
-static inline void dio_zero_block(struct dio *dio, struct dio_submit *sdio,
+static void dio_zero_block(struct dio *dio, struct dio_submit *sdio,
 		int end, struct buffer_head *map_bh)
 {
 	unsigned dio_blocks_per_fs_block;
-- 
1.8.1.4