2016-03-13 21:00:53

by Denys Vlasenko

[permalink] [raw]
Subject: [PATCH] direct-io: Deinline submit_page_section, save 5304 bytes

This function compiles to 2628 bytes, 2 callsites

text data bss dec hex filename
15197 16 0 15213 3b6d direct-io.o.before1
9655 16 0 9671 25c7 direct-io.o

Signed-off-by: Denys Vlasenko <[email protected]>
CC: Al Viro <[email protected]>
CC: Jens Axboe <[email protected]>
CC: Christoph Hellwig <[email protected]>
CC: [email protected]
---
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 d6a9012..21ec685 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -774,7 +774,7 @@ out:
* If that doesn't work out then we put the old page into the bio and add this
* page to the dio instead.
*/
-static inline int
+static int
submit_page_section(struct dio *dio, struct dio_submit *sdio, struct page *page,
unsigned offset, unsigned len, sector_t blocknr,
struct buffer_head *map_bh)
--
1.8.1.4


2016-03-13 21:01:05

by Denys Vlasenko

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

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 <[email protected]>
CC: Al Viro <[email protected]>
CC: Jens Axboe <[email protected]>
CC: Christoph Hellwig <[email protected]>
CC: [email protected]
---
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

2016-03-13 21:01:13

by Denys Vlasenko

[permalink] [raw]
Subject: [PATCH] direct-io: Deinline dio_send_cur_page, save ~2000 bytes

This function compiles to 1070 bytes, 2 callsites

text data bss dec hex filename
9559 16 0 9575 2567 direct-io.o.before3
7587 16 0 7603 1db3 direct-io.o

Signed-off-by: Denys Vlasenko <[email protected]>
CC: Al Viro <[email protected]>
CC: Jens Axboe <[email protected]>
CC: Christoph Hellwig <[email protected]>
CC: [email protected]
---
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 729c902..469baf7 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -710,7 +710,7 @@ static inline int dio_bio_add_page(struct dio_submit *sdio)
* The caller of this function is responsible for removing cur_page from the
* dio, and for dropping the refcount which came from that presence.
*/
-static inline int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio,
+static int dio_send_cur_page(struct dio *dio, struct dio_submit *sdio,
struct buffer_head *map_bh)
{
int ret = 0;
--
1.8.1.4

2016-03-13 21:36:34

by Denys Vlasenko

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

On Sun, Mar 13, 2016 at 10:00 PM, Denys Vlasenko <[email protected]> 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.

2016-03-14 01:07:20

by Ming Lei

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

On Mon, Mar 14, 2016 at 5:36 AM, Denys Vlasenko
<[email protected]> wrote:
> On Sun, Mar 13, 2016 at 10:00 PM, Denys Vlasenko <[email protected]> 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

2016-03-14 14:36:14

by Jeff Moyer

[permalink] [raw]
Subject: Re: [PATCH] direct-io: Deinline submit_page_section, save 5304 bytes

Denys Vlasenko <[email protected]> writes:

> This function compiles to 2628 bytes, 2 callsites
>
> text data bss dec hex filename
> 15197 16 0 15213 3b6d direct-io.o.before1
> 9655 16 0 9671 25c7 direct-io.o

And what is the consequence for performance? If you look at the git
history for this file, you'll find the commit that introduced the
inline:

commit ba253fbf6d3502c54e1ac8792e7ac8290a1f5b8d
Author: Andi Kleen <[email protected]>
Date: Mon Aug 1 21:38:08 2011 -0700

direct-io: inline the complete submission path

Add inlines to all the submission path functions. While this increases
code size it also gives gcc a lot of optimization opportunities
in this critical hotpath.

...

So you're essentially undoing that work.

NAK

Cheers,
Jeff

2016-03-14 14:37:32

by Jeff Moyer

[permalink] [raw]
Subject: Re: [PATCH] direct-io: Deinline dio_send_cur_page, save ~2000 bytes

Denys Vlasenko <[email protected]> writes:

> This function compiles to 1070 bytes, 2 callsites
>
> text data bss dec hex filename
> 9559 16 0 9575 2567 direct-io.o.before3
> 7587 16 0 7603 1db3 direct-io.o

NAK

2016-03-14 14:37:19

by Jeff Moyer

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

Denys Vlasenko <[email protected]> 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