[PATCH 0/2] Stack usage reduction

[PATCH 0/2] Stack usage reduction

[David Sterba]

Hi,

using the gcc option -fstack-usage I measured the stack usage and tried to get
rid of the worst offenders. The best improvement was in create_subvol where we
stored a 400B+ structure on stack, but otherwise it's not always clear why the
stack usage is that high. Most functions consume less than 300 bytes, and this
number can account for inlined functions or other invisible compiler
optimization magic.

A few samples of what's left:

scrub.c:3056:31:scrub_stripe                                         568  static
volumes.c:3984:12:btrfs_uuid_scan_kthread                            536  static
scrub.c:2834:31:scrub_raid56_parity                                  384  static
ioctl.c:5225:12:btrfs_ioctl_set_fslabel                              304  static
ioctl.c:1261:5:btrfs_defrag_file                                     304  static
tree-log.c:3593:21:copy_items                                        288  dynamic,bounded
ioctl.c:5202:12:btrfs_ioctl_get_fslabel                              288  static
ioctl.c:3457:12:btrfs_clone                                          288  static
extent_io.c:2873:12:__do_readpage                                    288  static
file.c:691:5:__btrfs_drop_extents                                    272  static
file.c:2646:13:btrfs_fallocate                                       272  static
extent-tree.c:2469:21:__btrfs_run_delayed_refs                       272  static
extent_io.c:3779:5:btree_write_cache_pages                           272  static
extent_io.c:1730:6:extent_clear_unlock_delalloc                      272  static
tree-log.c:4432:12:btrfs_log_inode                                   264  dynamic,bounded
tree-log.c:578:21:replay_one_extent                                  256  static
transaction.c:1322:21:create_pending_snapshot                        256  static
ioctl.c:434:21:create_subvol                                         256  static
inode.c:1221:21:run_delalloc_nocow                                   256  static
extent_io.c:4145:5:extent_readpages                                  256  static
tree-log.c:4126:12:btrfs_log_changed_extents                         248  dynamic,bounded
relocation.c:680:22:build_backref_tree                               248  static
inode.c:4287:5:btrfs_truncate_inode_items                            248  static
extent_io.c:3312:31:__extent_writepage_io                            248  static
volumes.c:2948:12:insert_balance_item                                240  static
relocation.c:1762:5:replace_path                                     240  static
...

More detailed info would be needed to decide whether it's worth to reshuffle
the stack variables, from what I've seen it would make the code readability
worse, so I've stopped.

----------------------------------------------------------------
The following changes since commit bb7ab3b92e46da06b580c6f83abe7894dc449cca:

  btrfs: Fix misspellings in comments. (2016-03-14 15:05:02 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git dev/stack-reduce

for you to fetch changes up to 355ef15a6eafc0cafd49b049d7173040adb44f61:

  btrfs: reuse existing variable in scrub_stripe, reduce stack usage (2016-03-24 18:06:34 +0100)

----------------------------------------------------------------
David Sterba (2):
      btrfs: use dynamic allocation for root item in create_subvol
      btrfs: reuse existing variable in scrub_stripe, reduce stack usage

 fs/btrfs/ioctl.c | 49 ++++++++++++++++++++++++++-----------------------
 fs/btrfs/scrub.c | 19 +++++++++----------
 2 files changed, 35 insertions(+), 33 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-

[PATCH 1/2] btrfs: use dynamic allocation for root item in create_subvol

[David Sterba]

The size of root item is more than 400 bytes, which is quite a lot of
stack space. As we do IO from inside the subvolume ioctls, we should
keep the stack usage low in case the filesystem is on top of other
layers (NFS, device mapper, iscsi, etc).

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 49 ++++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 053e677839fe..0be13b9c53d9 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -439,7 +439,7 @@ static noinline int create_subvol(struct inode *dir,
 {
 	struct btrfs_trans_handle *trans;
 	struct btrfs_key key;
-	struct btrfs_root_item root_item;
+	struct btrfs_root_item *root_item;
 	struct btrfs_inode_item *inode_item;
 	struct extent_buffer *leaf;
 	struct btrfs_root *root = BTRFS_I(dir)->root;
@@ -455,6 +455,10 @@ static noinline int create_subvol(struct inode *dir,
 	u64 qgroup_reserved;
 	uuid_le new_uuid;
 
+	root_item = kzalloc(sizeof(*root_item), GFP_KERNEL);
+	if (!root_item)
+		return -ENOMEM;
+
 	ret = btrfs_find_free_objectid(root->fs_info->tree_root, &objectid);
 	if (ret)
 		return ret;
@@ -509,47 +513,45 @@ static noinline int create_subvol(struct inode *dir,
 			    BTRFS_UUID_SIZE);
 	btrfs_mark_buffer_dirty(leaf);
 
-	memset(&root_item, 0, sizeof(root_item));
-
-	inode_item = &root_item.inode;
+	inode_item = &root_item->inode;
 	btrfs_set_stack_inode_generation(inode_item, 1);
 	btrfs_set_stack_inode_size(inode_item, 3);
 	btrfs_set_stack_inode_nlink(inode_item, 1);
 	btrfs_set_stack_inode_nbytes(inode_item, root->nodesize);
 	btrfs_set_stack_inode_mode(inode_item, S_IFDIR | 0755);
 
-	btrfs_set_root_flags(&root_item, 0);
-	btrfs_set_root_limit(&root_item, 0);
+	btrfs_set_root_flags(root_item, 0);
+	btrfs_set_root_limit(root_item, 0);
 	btrfs_set_stack_inode_flags(inode_item, BTRFS_INODE_ROOT_ITEM_INIT);
 
-	btrfs_set_root_bytenr(&root_item, leaf->start);
-	btrfs_set_root_generation(&root_item, trans->transid);
-	btrfs_set_root_level(&root_item, 0);
-	btrfs_set_root_refs(&root_item, 1);
-	btrfs_set_root_used(&root_item, leaf->len);
-	btrfs_set_root_last_snapshot(&root_item, 0);
+	btrfs_set_root_bytenr(root_item, leaf->start);
+	btrfs_set_root_generation(root_item, trans->transid);
+	btrfs_set_root_level(root_item, 0);
+	btrfs_set_root_refs(root_item, 1);
+	btrfs_set_root_used(root_item, leaf->len);
+	btrfs_set_root_last_snapshot(root_item, 0);
 
-	btrfs_set_root_generation_v2(&root_item,
-			btrfs_root_generation(&root_item));
+	btrfs_set_root_generation_v2(root_item,
+			btrfs_root_generation(root_item));
 	uuid_le_gen(&new_uuid);
-	memcpy(root_item.uuid, new_uuid.b, BTRFS_UUID_SIZE);
-	btrfs_set_stack_timespec_sec(&root_item.otime, cur_time.tv_sec);
-	btrfs_set_stack_timespec_nsec(&root_item.otime, cur_time.tv_nsec);
-	root_item.ctime = root_item.otime;
-	btrfs_set_root_ctransid(&root_item, trans->transid);
-	btrfs_set_root_otransid(&root_item, trans->transid);
+	memcpy(root_item->uuid, new_uuid.b, BTRFS_UUID_SIZE);
+	btrfs_set_stack_timespec_sec(&root_item->otime, cur_time.tv_sec);
+	btrfs_set_stack_timespec_nsec(&root_item->otime, cur_time.tv_nsec);
+	root_item->ctime = root_item->otime;
+	btrfs_set_root_ctransid(root_item, trans->transid);
+	btrfs_set_root_otransid(root_item, trans->transid);
 
 	btrfs_tree_unlock(leaf);
 	free_extent_buffer(leaf);
 	leaf = NULL;
 
-	btrfs_set_root_dirid(&root_item, new_dirid);
+	btrfs_set_root_dirid(root_item, new_dirid);
 
 	key.objectid = objectid;
 	key.offset = 0;
 	key.type = BTRFS_ROOT_ITEM_KEY;
 	ret = btrfs_insert_root(trans, root->fs_info->tree_root, &key,
-				&root_item);
+				root_item);
 	if (ret)
 		goto fail;
 
@@ -601,12 +603,13 @@ static noinline int create_subvol(struct inode *dir,
 	BUG_ON(ret);
 
 	ret = btrfs_uuid_tree_add(trans, root->fs_info->uuid_root,
-				  root_item.uuid, BTRFS_UUID_KEY_SUBVOL,
+				  root_item->uuid, BTRFS_UUID_KEY_SUBVOL,
 				  objectid);
 	if (ret)
 		btrfs_abort_transaction(trans, root, ret);
 
 fail:
+	kfree(root_item);
 	trans->block_rsv = NULL;
 	trans->bytes_reserved = 0;
 	btrfs_subvolume_release_metadata(root, &block_rsv, qgroup_reserved);
-- 
2.7.1

Re: [PATCH 1/2] btrfs: use dynamic allocation for root item in create_subvol

[Tsutomu Itoh]

On 2016/04/12 3:04, David Sterba wrote:
> The size of root item is more than 400 bytes, which is quite a lot of
> stack space. As we do IO from inside the subvolume ioctls, we should
> keep the stack usage low in case the filesystem is on top of other
> layers (NFS, device mapper, iscsi, etc).
> 
> Signed-off-by: David Sterba <dsterba@suse.com>
> ---
>   fs/btrfs/ioctl.c | 49 ++++++++++++++++++++++++++-----------------------
>   1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 053e677839fe..0be13b9c53d9 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -439,7 +439,7 @@ static noinline int create_subvol(struct inode *dir,
>   {
>   	struct btrfs_trans_handle *trans;
>   	struct btrfs_key key;
> -	struct btrfs_root_item root_item;
> +	struct btrfs_root_item *root_item;
>   	struct btrfs_inode_item *inode_item;
>   	struct extent_buffer *leaf;
>   	struct btrfs_root *root = BTRFS_I(dir)->root;
> @@ -455,6 +455,10 @@ static noinline int create_subvol(struct inode *dir,
>   	u64 qgroup_reserved;
>   	uuid_le new_uuid;
>   
> +	root_item = kzalloc(sizeof(*root_item), GFP_KERNEL);
> +	if (!root_item)
> +		return -ENOMEM;
> +
>   	ret = btrfs_find_free_objectid(root->fs_info->tree_root, &objectid);
>   	if (ret)
>   		return ret;

'kfree(root_item)' is necessary here and other 'return'.

Thanks,
Tsutomu

> @@ -509,47 +513,45 @@ static noinline int create_subvol(struct inode *dir,
>   			    BTRFS_UUID_SIZE);
>   	btrfs_mark_buffer_dirty(leaf);
>   
> -	memset(&root_item, 0, sizeof(root_item));
> -
> -	inode_item = &root_item.inode;
> +	inode_item = &root_item->inode;
>   	btrfs_set_stack_inode_generation(inode_item, 1);
>   	btrfs_set_stack_inode_size(inode_item, 3);
>   	btrfs_set_stack_inode_nlink(inode_item, 1);
>   	btrfs_set_stack_inode_nbytes(inode_item, root->nodesize);
>   	btrfs_set_stack_inode_mode(inode_item, S_IFDIR | 0755);
>   
> -	btrfs_set_root_flags(&root_item, 0);
> -	btrfs_set_root_limit(&root_item, 0);
> +	btrfs_set_root_flags(root_item, 0);
> +	btrfs_set_root_limit(root_item, 0);
>   	btrfs_set_stack_inode_flags(inode_item, BTRFS_INODE_ROOT_ITEM_INIT);
>   
> -	btrfs_set_root_bytenr(&root_item, leaf->start);
> -	btrfs_set_root_generation(&root_item, trans->transid);
> -	btrfs_set_root_level(&root_item, 0);
> -	btrfs_set_root_refs(&root_item, 1);
> -	btrfs_set_root_used(&root_item, leaf->len);
> -	btrfs_set_root_last_snapshot(&root_item, 0);
> +	btrfs_set_root_bytenr(root_item, leaf->start);
> +	btrfs_set_root_generation(root_item, trans->transid);
> +	btrfs_set_root_level(root_item, 0);
> +	btrfs_set_root_refs(root_item, 1);
> +	btrfs_set_root_used(root_item, leaf->len);
> +	btrfs_set_root_last_snapshot(root_item, 0);
>   
> -	btrfs_set_root_generation_v2(&root_item,
> -			btrfs_root_generation(&root_item));
> +	btrfs_set_root_generation_v2(root_item,
> +			btrfs_root_generation(root_item));
>   	uuid_le_gen(&new_uuid);
> -	memcpy(root_item.uuid, new_uuid.b, BTRFS_UUID_SIZE);
> -	btrfs_set_stack_timespec_sec(&root_item.otime, cur_time.tv_sec);
> -	btrfs_set_stack_timespec_nsec(&root_item.otime, cur_time.tv_nsec);
> -	root_item.ctime = root_item.otime;
> -	btrfs_set_root_ctransid(&root_item, trans->transid);
> -	btrfs_set_root_otransid(&root_item, trans->transid);
> +	memcpy(root_item->uuid, new_uuid.b, BTRFS_UUID_SIZE);
> +	btrfs_set_stack_timespec_sec(&root_item->otime, cur_time.tv_sec);
> +	btrfs_set_stack_timespec_nsec(&root_item->otime, cur_time.tv_nsec);
> +	root_item->ctime = root_item->otime;
> +	btrfs_set_root_ctransid(root_item, trans->transid);
> +	btrfs_set_root_otransid(root_item, trans->transid);
>   
>   	btrfs_tree_unlock(leaf);
>   	free_extent_buffer(leaf);
>   	leaf = NULL;
>   
> -	btrfs_set_root_dirid(&root_item, new_dirid);
> +	btrfs_set_root_dirid(root_item, new_dirid);
>   
>   	key.objectid = objectid;
>   	key.offset = 0;
>   	key.type = BTRFS_ROOT_ITEM_KEY;
>   	ret = btrfs_insert_root(trans, root->fs_info->tree_root, &key,
> -				&root_item);
> +				root_item);
>   	if (ret)
>   		goto fail;
>   
> @@ -601,12 +603,13 @@ static noinline int create_subvol(struct inode *dir,
>   	BUG_ON(ret);
>   
>   	ret = btrfs_uuid_tree_add(trans, root->fs_info->uuid_root,
> -				  root_item.uuid, BTRFS_UUID_KEY_SUBVOL,
> +				  root_item->uuid, BTRFS_UUID_KEY_SUBVOL,
>   				  objectid);
>   	if (ret)
>   		btrfs_abort_transaction(trans, root, ret);
>   
>   fail:
> +	kfree(root_item);
>   	trans->block_rsv = NULL;
>   	trans->bytes_reserved = 0;
>   	btrfs_subvolume_release_metadata(root, &block_rsv, qgroup_reserved);
> 

[PATCH v2] btrfs: use dynamic allocation for root item in create_subvol

[David Sterba]

The size of root item is more than 400 bytes, which is quite a lot of
stack space. As we do IO from inside the subvolume ioctls, we should
keep the stack usage low in case the filesystem is on top of other
layers (NFS, device mapper, iscsi, etc).

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/ioctl.c | 65 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 28 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 053e677839fe..9a63fe07bc2e 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -439,7 +439,7 @@ static noinline int create_subvol(struct inode *dir,
 {
 	struct btrfs_trans_handle *trans;
 	struct btrfs_key key;
-	struct btrfs_root_item root_item;
+	struct btrfs_root_item *root_item;
 	struct btrfs_inode_item *inode_item;
 	struct extent_buffer *leaf;
 	struct btrfs_root *root = BTRFS_I(dir)->root;
@@ -455,16 +455,22 @@ static noinline int create_subvol(struct inode *dir,
 	u64 qgroup_reserved;
 	uuid_le new_uuid;
 
+	root_item = kzalloc(sizeof(*root_item), GFP_KERNEL);
+	if (!root_item)
+		return -ENOMEM;
+
 	ret = btrfs_find_free_objectid(root->fs_info->tree_root, &objectid);
 	if (ret)
-		return ret;
+		goto fail_free;
 
 	/*
 	 * Don't create subvolume whose level is not zero. Or qgroup will be
 	 * screwed up since it assume subvolme qgroup's level to be 0.
 	 */
-	if (btrfs_qgroup_level(objectid))
-		return -ENOSPC;
+	if (btrfs_qgroup_level(objectid)) {
+		ret = -ENOSPC;
+		goto fail_free;
+	}
 
 	btrfs_init_block_rsv(&block_rsv, BTRFS_BLOCK_RSV_TEMP);
 	/*
@@ -474,14 +480,14 @@ static noinline int create_subvol(struct inode *dir,
 	ret = btrfs_subvolume_reserve_metadata(root, &block_rsv,
 					       8, &qgroup_reserved, false);
 	if (ret)
-		return ret;
+		goto fail_free;
 
 	trans = btrfs_start_transaction(root, 0);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
 		btrfs_subvolume_release_metadata(root, &block_rsv,
 						 qgroup_reserved);
-		return ret;
+		goto fail_free;
 	}
 	trans->block_rsv = &block_rsv;
 	trans->bytes_reserved = block_rsv.size;
@@ -509,47 +515,45 @@ static noinline int create_subvol(struct inode *dir,
 			    BTRFS_UUID_SIZE);
 	btrfs_mark_buffer_dirty(leaf);
 
-	memset(&root_item, 0, sizeof(root_item));
-
-	inode_item = &root_item.inode;
+	inode_item = &root_item->inode;
 	btrfs_set_stack_inode_generation(inode_item, 1);
 	btrfs_set_stack_inode_size(inode_item, 3);
 	btrfs_set_stack_inode_nlink(inode_item, 1);
 	btrfs_set_stack_inode_nbytes(inode_item, root->nodesize);
 	btrfs_set_stack_inode_mode(inode_item, S_IFDIR | 0755);
 
-	btrfs_set_root_flags(&root_item, 0);
-	btrfs_set_root_limit(&root_item, 0);
+	btrfs_set_root_flags(root_item, 0);
+	btrfs_set_root_limit(root_item, 0);
 	btrfs_set_stack_inode_flags(inode_item, BTRFS_INODE_ROOT_ITEM_INIT);
 
-	btrfs_set_root_bytenr(&root_item, leaf->start);
-	btrfs_set_root_generation(&root_item, trans->transid);
-	btrfs_set_root_level(&root_item, 0);
-	btrfs_set_root_refs(&root_item, 1);
-	btrfs_set_root_used(&root_item, leaf->len);
-	btrfs_set_root_last_snapshot(&root_item, 0);
+	btrfs_set_root_bytenr(root_item, leaf->start);
+	btrfs_set_root_generation(root_item, trans->transid);
+	btrfs_set_root_level(root_item, 0);
+	btrfs_set_root_refs(root_item, 1);
+	btrfs_set_root_used(root_item, leaf->len);
+	btrfs_set_root_last_snapshot(root_item, 0);
 
-	btrfs_set_root_generation_v2(&root_item,
-			btrfs_root_generation(&root_item));
+	btrfs_set_root_generation_v2(root_item,
+			btrfs_root_generation(root_item));
 	uuid_le_gen(&new_uuid);
-	memcpy(root_item.uuid, new_uuid.b, BTRFS_UUID_SIZE);
-	btrfs_set_stack_timespec_sec(&root_item.otime, cur_time.tv_sec);
-	btrfs_set_stack_timespec_nsec(&root_item.otime, cur_time.tv_nsec);
-	root_item.ctime = root_item.otime;
-	btrfs_set_root_ctransid(&root_item, trans->transid);
-	btrfs_set_root_otransid(&root_item, trans->transid);
+	memcpy(root_item->uuid, new_uuid.b, BTRFS_UUID_SIZE);
+	btrfs_set_stack_timespec_sec(&root_item->otime, cur_time.tv_sec);
+	btrfs_set_stack_timespec_nsec(&root_item->otime, cur_time.tv_nsec);
+	root_item->ctime = root_item->otime;
+	btrfs_set_root_ctransid(root_item, trans->transid);
+	btrfs_set_root_otransid(root_item, trans->transid);
 
 	btrfs_tree_unlock(leaf);
 	free_extent_buffer(leaf);
 	leaf = NULL;
 
-	btrfs_set_root_dirid(&root_item, new_dirid);
+	btrfs_set_root_dirid(root_item, new_dirid);
 
 	key.objectid = objectid;
 	key.offset = 0;
 	key.type = BTRFS_ROOT_ITEM_KEY;
 	ret = btrfs_insert_root(trans, root->fs_info->tree_root, &key,
-				&root_item);
+				root_item);
 	if (ret)
 		goto fail;
 
@@ -601,12 +605,13 @@ static noinline int create_subvol(struct inode *dir,
 	BUG_ON(ret);
 
 	ret = btrfs_uuid_tree_add(trans, root->fs_info->uuid_root,
-				  root_item.uuid, BTRFS_UUID_KEY_SUBVOL,
+				  root_item->uuid, BTRFS_UUID_KEY_SUBVOL,
 				  objectid);
 	if (ret)
 		btrfs_abort_transaction(trans, root, ret);
 
 fail:
+	kfree(root_item);
 	trans->block_rsv = NULL;
 	trans->bytes_reserved = 0;
 	btrfs_subvolume_release_metadata(root, &block_rsv, qgroup_reserved);
@@ -629,6 +634,10 @@ static noinline int create_subvol(struct inode *dir,
 		d_instantiate(dentry, inode);
 	}
 	return ret;
+
+fail_free:
+	kfree(root_item);
+	return ret;
 }
 
 static void btrfs_wait_for_no_snapshoting_writes(struct btrfs_root *root)
-- 
2.7.1

Re: [PATCH v2] btrfs: use dynamic allocation for root item in create_subvol

[Tsutomu Itoh]

On 2016/04/25 20:18, David Sterba wrote:
> The size of root item is more than 400 bytes, which is quite a lot of
> stack space. As we do IO from inside the subvolume ioctls, we should
> keep the stack usage low in case the filesystem is on top of other
> layers (NFS, device mapper, iscsi, etc).
> 
> Signed-off-by: David Sterba <dsterba@suse.com>

Looks good to me.

Reviewed-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>

> ---
>   fs/btrfs/ioctl.c | 65 ++++++++++++++++++++++++++++++++------------------------
>   1 file changed, 37 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 053e677839fe..9a63fe07bc2e 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -439,7 +439,7 @@ static noinline int create_subvol(struct inode *dir,
>   {
>   	struct btrfs_trans_handle *trans;
>   	struct btrfs_key key;
> -	struct btrfs_root_item root_item;
> +	struct btrfs_root_item *root_item;
>   	struct btrfs_inode_item *inode_item;
>   	struct extent_buffer *leaf;
>   	struct btrfs_root *root = BTRFS_I(dir)->root;
> @@ -455,16 +455,22 @@ static noinline int create_subvol(struct inode *dir,
>   	u64 qgroup_reserved;
>   	uuid_le new_uuid;
>   
> +	root_item = kzalloc(sizeof(*root_item), GFP_KERNEL);
> +	if (!root_item)
> +		return -ENOMEM;
> +
>   	ret = btrfs_find_free_objectid(root->fs_info->tree_root, &objectid);
>   	if (ret)
> -		return ret;
> +		goto fail_free;
>   
>   	/*
>   	 * Don't create subvolume whose level is not zero. Or qgroup will be
>   	 * screwed up since it assume subvolme qgroup's level to be 0.
>   	 */
> -	if (btrfs_qgroup_level(objectid))
> -		return -ENOSPC;
> +	if (btrfs_qgroup_level(objectid)) {
> +		ret = -ENOSPC;
> +		goto fail_free;
> +	}
>   
>   	btrfs_init_block_rsv(&block_rsv, BTRFS_BLOCK_RSV_TEMP);
>   	/*
> @@ -474,14 +480,14 @@ static noinline int create_subvol(struct inode *dir,
>   	ret = btrfs_subvolume_reserve_metadata(root, &block_rsv,
>   					       8, &qgroup_reserved, false);
>   	if (ret)
> -		return ret;
> +		goto fail_free;
>   
>   	trans = btrfs_start_transaction(root, 0);
>   	if (IS_ERR(trans)) {
>   		ret = PTR_ERR(trans);
>   		btrfs_subvolume_release_metadata(root, &block_rsv,
>   						 qgroup_reserved);
> -		return ret;
> +		goto fail_free;
>   	}
>   	trans->block_rsv = &block_rsv;
>   	trans->bytes_reserved = block_rsv.size;
> @@ -509,47 +515,45 @@ static noinline int create_subvol(struct inode *dir,
>   			    BTRFS_UUID_SIZE);
>   	btrfs_mark_buffer_dirty(leaf);
>   
> -	memset(&root_item, 0, sizeof(root_item));
> -
> -	inode_item = &root_item.inode;
> +	inode_item = &root_item->inode;
>   	btrfs_set_stack_inode_generation(inode_item, 1);
>   	btrfs_set_stack_inode_size(inode_item, 3);
>   	btrfs_set_stack_inode_nlink(inode_item, 1);
>   	btrfs_set_stack_inode_nbytes(inode_item, root->nodesize);
>   	btrfs_set_stack_inode_mode(inode_item, S_IFDIR | 0755);
>   
> -	btrfs_set_root_flags(&root_item, 0);
> -	btrfs_set_root_limit(&root_item, 0);
> +	btrfs_set_root_flags(root_item, 0);
> +	btrfs_set_root_limit(root_item, 0);
>   	btrfs_set_stack_inode_flags(inode_item, BTRFS_INODE_ROOT_ITEM_INIT);
>   
> -	btrfs_set_root_bytenr(&root_item, leaf->start);
> -	btrfs_set_root_generation(&root_item, trans->transid);
> -	btrfs_set_root_level(&root_item, 0);
> -	btrfs_set_root_refs(&root_item, 1);
> -	btrfs_set_root_used(&root_item, leaf->len);
> -	btrfs_set_root_last_snapshot(&root_item, 0);
> +	btrfs_set_root_bytenr(root_item, leaf->start);
> +	btrfs_set_root_generation(root_item, trans->transid);
> +	btrfs_set_root_level(root_item, 0);
> +	btrfs_set_root_refs(root_item, 1);
> +	btrfs_set_root_used(root_item, leaf->len);
> +	btrfs_set_root_last_snapshot(root_item, 0);
>   
> -	btrfs_set_root_generation_v2(&root_item,
> -			btrfs_root_generation(&root_item));
> +	btrfs_set_root_generation_v2(root_item,
> +			btrfs_root_generation(root_item));
>   	uuid_le_gen(&new_uuid);
> -	memcpy(root_item.uuid, new_uuid.b, BTRFS_UUID_SIZE);
> -	btrfs_set_stack_timespec_sec(&root_item.otime, cur_time.tv_sec);
> -	btrfs_set_stack_timespec_nsec(&root_item.otime, cur_time.tv_nsec);
> -	root_item.ctime = root_item.otime;
> -	btrfs_set_root_ctransid(&root_item, trans->transid);
> -	btrfs_set_root_otransid(&root_item, trans->transid);
> +	memcpy(root_item->uuid, new_uuid.b, BTRFS_UUID_SIZE);
> +	btrfs_set_stack_timespec_sec(&root_item->otime, cur_time.tv_sec);
> +	btrfs_set_stack_timespec_nsec(&root_item->otime, cur_time.tv_nsec);
> +	root_item->ctime = root_item->otime;
> +	btrfs_set_root_ctransid(root_item, trans->transid);
> +	btrfs_set_root_otransid(root_item, trans->transid);
>   
>   	btrfs_tree_unlock(leaf);
>   	free_extent_buffer(leaf);
>   	leaf = NULL;
>   
> -	btrfs_set_root_dirid(&root_item, new_dirid);
> +	btrfs_set_root_dirid(root_item, new_dirid);
>   
>   	key.objectid = objectid;
>   	key.offset = 0;
>   	key.type = BTRFS_ROOT_ITEM_KEY;
>   	ret = btrfs_insert_root(trans, root->fs_info->tree_root, &key,
> -				&root_item);
> +				root_item);
>   	if (ret)
>   		goto fail;
>   
> @@ -601,12 +605,13 @@ static noinline int create_subvol(struct inode *dir,
>   	BUG_ON(ret);
>   
>   	ret = btrfs_uuid_tree_add(trans, root->fs_info->uuid_root,
> -				  root_item.uuid, BTRFS_UUID_KEY_SUBVOL,
> +				  root_item->uuid, BTRFS_UUID_KEY_SUBVOL,
>   				  objectid);
>   	if (ret)
>   		btrfs_abort_transaction(trans, root, ret);
>   
>   fail:
> +	kfree(root_item);
>   	trans->block_rsv = NULL;
>   	trans->bytes_reserved = 0;
>   	btrfs_subvolume_release_metadata(root, &block_rsv, qgroup_reserved);
> @@ -629,6 +634,10 @@ static noinline int create_subvol(struct inode *dir,
>   		d_instantiate(dentry, inode);
>   	}
>   	return ret;
> +
> +fail_free:
> +	kfree(root_item);
> +	return ret;
>   }
>   
>   static void btrfs_wait_for_no_snapshoting_writes(struct btrfs_root *root)
> 

[PATCH 2/2] btrfs: reuse existing variable in scrub_stripe, reduce stack usage

[David Sterba]

The key variable occupies 17 bytes, the key_start is used once, we can
simply reuse existing 'key' for that purpose. As the key is not a simple
type, compiler doest not do it on itself.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/scrub.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
index 39dbdcbf4d13..07db452e4a15 100644
--- a/fs/btrfs/scrub.c
+++ b/fs/btrfs/scrub.c
@@ -3070,7 +3070,6 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 	int slot;
 	u64 nstripes;
 	struct extent_buffer *l;
-	struct btrfs_key key;
 	u64 physical;
 	u64 logical;
 	u64 logic_end;
@@ -3079,7 +3078,7 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 	int mirror_num;
 	struct reada_control *reada1;
 	struct reada_control *reada2;
-	struct btrfs_key key_start;
+	struct btrfs_key key;
 	struct btrfs_key key_end;
 	u64 increment = map->stripe_len;
 	u64 offset;
@@ -3158,21 +3157,21 @@ static noinline_for_stack int scrub_stripe(struct scrub_ctx *sctx,
 	scrub_blocked_if_needed(fs_info);
 
 	/* FIXME it might be better to start readahead at commit root */
-	key_start.objectid = logical;
-	key_start.type = BTRFS_EXTENT_ITEM_KEY;
-	key_start.offset = (u64)0;
+	key.objectid = logical;
+	key.type = BTRFS_EXTENT_ITEM_KEY;
+	key.offset = (u64)0;
 	key_end.objectid = logic_end;
 	key_end.type = BTRFS_METADATA_ITEM_KEY;
 	key_end.offset = (u64)-1;
-	reada1 = btrfs_reada_add(root, &key_start, &key_end);
+	reada1 = btrfs_reada_add(root, &key, &key_end);
 
-	key_start.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
-	key_start.type = BTRFS_EXTENT_CSUM_KEY;
-	key_start.offset = logical;
+	key.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
+	key.type = BTRFS_EXTENT_CSUM_KEY;
+	key.offset = logical;
 	key_end.objectid = BTRFS_EXTENT_CSUM_OBJECTID;
 	key_end.type = BTRFS_EXTENT_CSUM_KEY;
 	key_end.offset = logic_end;
-	reada2 = btrfs_reada_add(csum_root, &key_start, &key_end);
+	reada2 = btrfs_reada_add(csum_root, &key, &key_end);
 
 	if (!IS_ERR(reada1))
 		btrfs_reada_wait(reada1);
-- 
2.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-