Skip to contentSkip to conversation

btrfs: reduce stack usage for btrfsic_process_written_block

PATCH In review Arnd Bergmann 3 participants 3 messages Jul 8, 2019 – Jul 24, 2019 Btrfs file system

lore
Share & tools
lore

Arnd Bergmann

author
#0 lore raw

PATCH 1/1 btrfs: reduce stack usage for btrfsic_process_written_block

btrfsic_process_written_block() cals btrfsic_process_metablock(), which has a fairly large stack usage due to the btrfsic_stack_frame variable. It also calls btrfsic_test_for_metadata(), which now needs several hundreds of bytes for its SHASH_DESC_ON_STACK().

In some configurations, we end up with both functions on the same stack, and gcc warns about the excessive stack usage that might cause the available stack space to run out:

fs/btrfs/check-integrity.c:1743:13: error: stack frame size of 1152 bytes in function 'btrfsic_process_written_block' [-Werror,-Wframe-larger-than=]

Marking both child functions as noinline_for_stack helps because this guarantees that the large variables are not on the same stack frame.

Fixes d5178578bcd4 ("btrfs: directly call into crypto framework for checksumming")
Signed-off-by Arnd Bergmann <[email protected]>
---
 fs/btrfs/check-integrity.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/btrfs/check-integrity.c +4 -3
index 81a9731959a9..0b52ab4cb964 100644
@@ -940,7 +940,7 @@ static void btrfsic_stack_frame_free(struct btrfsic_stack_frame *sf)
 	kfree(sf);
 }
 
-static int btrfsic_process_metablock(
+static noinline_for_stack int btrfsic_process_metablock(
 		struct btrfsic_state *state,
 		struct btrfsic_block *const first_block,
 		struct btrfsic_block_data_ctx *const first_block_ctx,

@@ -1706,8 +1706,9 @@ static void btrfsic_dump_database(struct btrfsic_state *state)
  * Test whether the disk block contains a tree block (leaf or node)
  * (note that this test fails for the super block)
  */
-static int btrfsic_test_for_metadata(struct btrfsic_state *state,
-				     char **datav, unsigned int num_pages)
+static noinline_for_stack int btrfsic_test_for_metadata(
+		struct btrfsic_state *state,
+		char **datav, unsigned int num_pages)
 {
 	struct btrfs_fs_info *fs_info = state->fs_info;
 	SHASH_DESC_ON_STACK(shash, fs_info->csum_shash);
Signature · 1 line
2.20.0
Reviewed-by on v1 1/1
#1 lore raw

Looks good,

Reviewed-by Johannes Thumshirn <[email protected]>

Thanks Arnd

Signature · 6 lines
Johannes Thumshirn                            SUSE Labs Filesystems
[email protected]                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
#2 lore raw
In reply to Arnd Bergmann
Quoted 9 lines · Arnd Bergmann

btrfsic_process_written_block() cals btrfsic_process_metablock(), which has a fairly large stack usage due to the btrfsic_stack_frame variable. It also calls btrfsic_test_for_metadata(), which now needs several hundreds of bytes for its SHASH_DESC_ON_STACK().

In some configurations, we end up with both functions on the same stack, and gcc warns about the excessive stack usage that might cause the available stack space to run out:

Quoted 8 lines

fs/btrfs/check-integrity.c:1743:13: error: stack frame size of 1152 bytes in function 'btrfsic_process_written_block' [-Werror,-Wframe-larger-than=]

Marking both child functions as noinline_for_stack helps because this guarantees that the large variables are not on the same stack frame.

Fixes d5178578bcd4 ("btrfs: directly call into crypto framework for checksumming")
Signed-off-by Arnd Bergmann <[email protected]>

Added to misc-next, thanks.