Linux Btrfs filesystem development
 help / color / mirror / Atom feed
* [PATCH] btrfs: make sure report_eb_range() is not inlined
@ 2026-04-24 15:17 fdmanana
  2026-04-27 14:30 ` David Sterba
  0 siblings, 1 reply; 2+ messages in thread
From: fdmanana @ 2026-04-24 15:17 UTC (permalink / raw)
  To: linux-btrfs

From: Filipe Manana <fdmanana@suse.com>

If report_rb_range() is inlined into its single caller (check_eb_range()),
we end up with a larger module size, which is undesirable and does not
provide any advantage since this code is for a cold path which we don't
expect to ever hit.

Add the noinline attribute to report_rb_range() and while at it also make
it return void as it always returns true.

Before this change (with gcc 14.2.0-19 from Debian):

  $ size fs/btrfs/btrfs.ko
     text	   data	    bss	    dec	    hex	filename
  2018267	 176232	  15592	2210091	 21b92b	fs/btrfs/btrfs.ko

After this change:

  $ size fs/btrfs/btrfs.ko
     text	   data	    bss	    dec	    hex	filename
  2017835	 176048	  15592	2209475	 21b6c3	fs/btrfs/btrfs.ko

Also, replacing the noinline with __cold, yields slighty worse results:

  $ size fs/btrfs/btrfs.ko
     text	   data	    bss	    dec	    hex	filename
  2017889	 176048	  15592	2209529	 21b6f9	fs/btrfs/btrfs.ko

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/extent_io.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 3d20e2d418ac..b6a92c9a17cf 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3976,15 +3976,14 @@ int read_extent_buffer_pages(struct extent_buffer *eb, int mirror_num,
 	return 0;
 }
 
-static bool report_eb_range(const struct extent_buffer *eb, unsigned long start,
-			    unsigned long len)
+/* Never inlined to decrease code size, as this is called in a cold path. */
+static noinline void report_eb_range(const struct extent_buffer *eb,
+				     unsigned long start, unsigned long len)
 {
 	btrfs_warn(eb->fs_info,
 		"access to eb bytenr %llu len %u out of range start %lu len %lu",
 		eb->start, eb->len, start, len);
 	DEBUG_WARN();
-
-	return true;
 }
 
 /*
@@ -4000,8 +3999,10 @@ static inline bool check_eb_range(const struct extent_buffer *eb,
 	unsigned long offset;
 
 	/* start, start + len should not go beyond eb->len nor overflow */
-	if (unlikely(check_add_overflow(start, len, &offset) || offset > eb->len))
-		return report_eb_range(eb, start, len);
+	if (unlikely(check_add_overflow(start, len, &offset) || offset > eb->len)) {
+		report_eb_range(eb, start, len);
+		return true;
+	}
 
 	return false;
 }
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] btrfs: make sure report_eb_range() is not inlined
  2026-04-24 15:17 [PATCH] btrfs: make sure report_eb_range() is not inlined fdmanana
@ 2026-04-27 14:30 ` David Sterba
  0 siblings, 0 replies; 2+ messages in thread
From: David Sterba @ 2026-04-27 14:30 UTC (permalink / raw)
  To: fdmanana; +Cc: linux-btrfs

On Fri, Apr 24, 2026 at 04:17:40PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> If report_rb_range() is inlined into its single caller (check_eb_range()),
> we end up with a larger module size, which is undesirable and does not
> provide any advantage since this code is for a cold path which we don't
> expect to ever hit.
> 
> Add the noinline attribute to report_rb_range() and while at it also make
> it return void as it always returns true.

With the 'bool' type the compiler can optimize the code by doing tail
call, in this case also the arguments match. I did a few checks how much
effects it has and there are some, eg. further specializations of
functions depending on the calling context and moving the call/jmp to
report out of the way.

> Before this change (with gcc 14.2.0-19 from Debian):
> 
>   $ size fs/btrfs/btrfs.ko
>      text	   data	    bss	    dec	    hex	filename
>   2018267	 176232	  15592	2210091	 21b92b	fs/btrfs/btrfs.ko
> 
> After this change:
> 
>   $ size fs/btrfs/btrfs.ko
>      text	   data	    bss	    dec	    hex	filename
>   2017835	 176048	  15592	2209475	 21b6c3	fs/btrfs/btrfs.ko
> 
> Also, replacing the noinline with __cold, yields slighty worse results:
> 
>   $ size fs/btrfs/btrfs.ko
>      text	   data	    bss	    dec	    hex	filename
>   2017889	 176048	  15592	2209529	 21b6f9	fs/btrfs/btrfs.ko

In the generated assembly seems that __cold is a bit different than
just the noinline. The __cold annotation also changes the placement of
the function to the cold section.

Out of all the combinations the noinline seems to have biggest effect,
namely on the stack usage. Cold or return value for tail calls is
proably too fine-grained for this function and not worth.

Reviewed-by: David Sterba <dsterba@suse.com>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-04-27 14:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-24 15:17 [PATCH] btrfs: make sure report_eb_range() is not inlined fdmanana
2026-04-27 14:30 ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox