Reduced impact of allocation failures during snapshot creation

PATCH New David Sterba 1 participant 4 messages Nov 10, 2015 Btrfs file system

Share & tools

David Sterba

author maintainer

2015-11-10 17:55 UTC #0 lore raw

cover Reduced impact of allocation failures during snapshot creation

Here's an improvement in allocation of temporary structures during snaphost creation, namely the root_item. The allocation is moved to the ioctl call, so it will fail early and not during the transaction commit. I've once hit it in practice.

It's aimed for 4.5 dev cycle and can be pulled from

git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git dev/prealloc-subvol

David Sterba (3):
btrfs: do an allocation earlier during snapshot creation
btrfs: allocate root item at snapshot ioctl time
btrfs: preallocate path for snapshot creation at ioctl time

 fs/btrfs/ioctl.c       | 31 ++++++++++++++++++++-----------
 fs/btrfs/transaction.c | 18 +++++++-----------
 fs/btrfs/transaction.h |  2 ++
 3 files changed, 29 insertions(+), 22 deletions(-)

Signature · 1 line

2.6.2

David Sterba

author maintainer

2015-11-10 17:55 UTC #1 lore raw

PATCH 1/3 btrfs: do an allocation earlier during snapshot creation

We can allocate pending_snapshot earlier and do not have to do cleanup in case of failure.

Signed-off-by David Sterba <[email protected]>

---
 fs/btrfs/ioctl.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

fs/btrfs/ioctl.c +9 ~~-11~~

index 8d20f3b1cab0..bf7d8ba7dd00 100644
@@ -655,22 +655,20 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 	if (!test_bit(BTRFS_ROOT_REF_COWS, &root->state))
 		return -EINVAL;
 
+	pending_snapshot = kzalloc(sizeof(*pending_snapshot), GFP_NOFS);
+	if (!pending_snapshot)
+		return -ENOMEM;
+
 	atomic_inc(&root->will_be_snapshoted);
 	smp_mb__after_atomic();
 	btrfs_wait_for_no_snapshoting_writes(root);
 
 	ret = btrfs_start_delalloc_inodes(root, 0);
 	if (ret)
-		goto out;
+		goto dec_and_free;
 
 	btrfs_wait_ordered_extents(root, -1);
 
-	pending_snapshot = kzalloc(sizeof(*pending_snapshot), GFP_NOFS);
-	if (!pending_snapshot) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
 	btrfs_init_block_rsv(&pending_snapshot->block_rsv,
 			     BTRFS_BLOCK_RSV_TEMP);
 	/*

@@ -686,7 +684,7 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 					&pending_snapshot->qgroup_reserved,
 					false);
 	if (ret)
-		goto free;
+		goto dec_and_free;
 
 	pending_snapshot->dentry = dentry;
 	pending_snapshot->root = root;

@@ -737,11 +735,11 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 	btrfs_subvolume_release_metadata(BTRFS_I(dir)->root,
 					 &pending_snapshot->block_rsv,
 					 pending_snapshot->qgroup_reserved);
-free:
-	kfree(pending_snapshot);
-out:
+dec_and_free:
 	if (atomic_dec_and_test(&root->will_be_snapshoted))
 		wake_up_atomic_t(&root->will_be_snapshoted);
+	kfree(pending_snapshot);
+
 	return ret;
 }

Signature · 6 lines

2.6.2

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

David Sterba

author maintainer

2015-11-10 17:55 UTC #2 lore raw

PATCH 2/3 btrfs: allocate root item at snapshot ioctl time

In reply to David Sterba

The actual snapshot creation is delayed until transaction commit. If we cannot get enough memory for the root item there, we have to fail the whole transaction commit which is bad. So we'll allocate the memory at the ioctl call and pass it along with the pending_snapshot struct. The potential ENOMEM will be returned to the caller of snapshot ioctl.

Signed-off-by David Sterba <[email protected]>

---
 fs/btrfs/ioctl.c       | 9 +++++++++
 fs/btrfs/transaction.c | 9 +++------
 fs/btrfs/transaction.h | 1 +
 3 files changed, 13 insertions(+), 6 deletions(-)

fs/btrfs/ioctl.c +9 -0

index bf7d8ba7dd00..040faa8eebc9 100644
@@ -659,6 +659,13 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 	if (!pending_snapshot)
 		return -ENOMEM;
 
+	pending_snapshot->root_item = kzalloc(sizeof(struct btrfs_root_item),
+			GFP_NOFS);
+	if (!pending_snapshot->root_item) {
+		ret = -ENOMEM;
+		goto free_pending;
+	}
+
 	atomic_inc(&root->will_be_snapshoted);
 	smp_mb__after_atomic();
 	btrfs_wait_for_no_snapshoting_writes(root);

@@ -738,6 +745,8 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 dec_and_free:
 	if (atomic_dec_and_test(&root->will_be_snapshoted))
 		wake_up_atomic_t(&root->will_be_snapshoted);
+free_pending:
+	kfree(pending_snapshot->root_item);
 	kfree(pending_snapshot);
 
 	return ret;

fs/btrfs/transaction.c +3 -6

index a5b06442f0bf..09129b65dd4b 100644
@@ -1320,11 +1320,8 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 		return 0;
 	}
 
-	new_root_item = kmalloc(sizeof(*new_root_item), GFP_NOFS);
-	if (!new_root_item) {
-		pending->error = -ENOMEM;
-		goto root_item_alloc_fail;
-	}
+	ASSERT(pending->root_item);
+	new_root_item = pending->root_item;
 
 	pending->error = btrfs_find_free_objectid(tree_root, &objectid);
 	if (pending->error)

@@ -1557,7 +1554,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	btrfs_clear_skip_qgroup(trans);
 no_free_objectid:
 	kfree(new_root_item);
-root_item_alloc_fail:
+	pending->root_item = NULL;
 	btrfs_free_path(path);
 	return ret;
 }

fs/btrfs/transaction.h +1 -0

index a994bb097ee5..a4e1c1a40880 100644
@@ -138,6 +138,7 @@ struct btrfs_pending_snapshot {
 	struct dentry *dentry;
 	struct inode *dir;
 	struct btrfs_root *root;
+	struct btrfs_root_item *root_item;
 	struct btrfs_root *snap;
 	struct btrfs_qgroup_inherit *inherit;
 	/* block reservation for the operation */

Signature · 6 lines

2.6.2

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

David Sterba

author maintainer

2015-11-10 17:55 UTC #3 lore raw

PATCH 3/3 btrfs: preallocate path for snapshot creation at ioctl time

In reply to David Sterba

We can also preallocate btrfs_path that's used during pending snapshot creation and avoid another late ENOMEM failure.

Signed-off-by David Sterba <[email protected]>

---
 fs/btrfs/ioctl.c       | 4 +++-
 fs/btrfs/transaction.c | 9 ++++-----
 fs/btrfs/transaction.h | 1 +
 3 files changed, 8 insertions(+), 6 deletions(-)

fs/btrfs/ioctl.c +3 -1

index 040faa8eebc9..d318971d3085 100644
@@ -661,7 +661,8 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 
 	pending_snapshot->root_item = kzalloc(sizeof(struct btrfs_root_item),
 			GFP_NOFS);
-	if (!pending_snapshot->root_item) {
+	pending_snapshot->path = btrfs_alloc_path();
+	if (!pending_snapshot->root_item || !pending_snapshot->path) {
 		ret = -ENOMEM;
 		goto free_pending;
 	}

@@ -747,6 +748,7 @@ static int create_snapshot(struct btrfs_root *root, struct inode *dir,
 		wake_up_atomic_t(&root->will_be_snapshoted);
 free_pending:
 	kfree(pending_snapshot->root_item);
+	btrfs_free_path(pending_snapshot->path);
 	kfree(pending_snapshot);
 
 	return ret;

fs/btrfs/transaction.c +4 -5

index 09129b65dd4b..732ad46bf015 100644
@@ -1314,11 +1314,8 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	u64 root_flags;
 	uuid_le new_uuid;
 
-	path = btrfs_alloc_path();
-	if (!path) {
-		pending->error = -ENOMEM;
-		return 0;
-	}
+	ASSERT(pending->path);
+	path = pending->path;
 
 	ASSERT(pending->root_item);
 	new_root_item = pending->root_item;

@@ -1556,6 +1553,8 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 	kfree(new_root_item);
 	pending->root_item = NULL;
 	btrfs_free_path(path);
+	pending->path = NULL;
+
 	return ret;
 }

fs/btrfs/transaction.h +1 -0

index a4e1c1a40880..9c94344fe204 100644
@@ -141,6 +141,7 @@ struct btrfs_pending_snapshot {
 	struct btrfs_root_item *root_item;
 	struct btrfs_root *snap;
 	struct btrfs_qgroup_inherit *inherit;
+	struct btrfs_path *path;
 	/* block reservation for the operation */
 	struct btrfs_block_rsv block_rsv;
 	u64 qgroup_reserved;

Signature · 6 lines

2.6.2

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