diff mbox

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

Message ID 7d013acc6e07b39bd224f9c0097775c0a8e138df.1460397148.git.dsterba@suse.com (mailing list archive)
State Superseded
Headers show

Commit Message

David Sterba April 11, 2016, 6:04 p.m. UTC 
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(-)

Comments

Tsutomu Itoh April 12, 2016, 12:15 a.m. UTC | #1 
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);
> 

--
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-info.html
diff mbox

Patch

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);