[v2,3/3] btrfs: add snapshot_lock to new_fs_root_args

Commit Message

robbieko Dec. 14, 2022, 2:11 a.m. UTC

From: Robbie Ko <[e-mail]>

[Issue]
When creating subvolume/snapshot, the transaction may
be abort due to -ENOMEM.

[Cause]
During creating a subvolume/snapshot, it is necessary
to allocate memory for Initializing fs root.
Therefore, it can only use GFP_NOFS to allocate memory
to avoid deadlock issues.
However, atomic allocation is required when processing
percpu_counter_init without GFP_KERNEL due to the unique
structure of percpu_counter.
In this situation, allocating memory for initializing
fs root may cause unexpected -ENOMEM when free memory
is low and causes btrfs transaction to abort.

[Fix]
So we add snapshot_lock into new_fs_root_args to allocate
the memory before holding a transaction handle.
This way, we can reduce the chances of -ENOMEM when
calling btrfs_init_fs_root.
Furthermore, it can avoid aborting a transaction and
turn the fs to RO mode.

Signed-off-by: Robbie Ko <[e-mail]>
---
 fs/btrfs/disk-io.c | 44 +++++++++++++++++++++++++++++++-------------
 fs/btrfs/disk-io.h |  2 ++
 2 files changed, 33 insertions(+), 13 deletions(-)

Patch

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index ed2ce2dfbfcd..7ba1a019f5b0 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1519,6 +1519,15 @@  struct btrfs_new_fs_root_args *btrfs_alloc_new_fs_root_args(void)
 	if (err)
 		goto error;
 
+	args->snapshot_lock = kzalloc(sizeof(*args->snapshot_lock), GFP_KERNEL);
+	if (!args->snapshot_lock) {
+		err = -ENOMEM;
+		goto error;
+	}
+	err = btrfs_drew_lock_init(args->snapshot_lock);
+	if (err)
+		goto error;
+
 	return args;
 
 error:
@@ -1530,6 +1539,10 @@  void btrfs_free_new_fs_root_args(struct btrfs_new_fs_root_args *args)
 {
 	if (!args)
 		return;
+	if (args->snapshot_lock) {
+		btrfs_drew_lock_destroy(args->snapshot_lock);
+		kfree(args->snapshot_lock);
+	}
 	if (args->anon_dev)
 		free_anon_bdev(args->anon_dev);
 	kfree(args);
@@ -1546,20 +1559,25 @@  static int btrfs_init_fs_root(struct btrfs_root *root,
 	int ret;
 	unsigned int nofs_flag;
 
-	root->snapshot_lock = kzalloc(sizeof(*root->snapshot_lock), GFP_NOFS);
-	if (!root->snapshot_lock) {
-		ret = -ENOMEM;
-		goto fail;
+	if (new_fs_root_args && new_fs_root_args->snapshot_lock) {
+		root->snapshot_lock = new_fs_root_args->snapshot_lock;
+		new_fs_root_args->snapshot_lock = NULL;
+	} else {
+		root->snapshot_lock = kzalloc(sizeof(*root->snapshot_lock), GFP_NOFS);
+		if (!root->snapshot_lock) {
+			ret = -ENOMEM;
+			goto fail;
+		}
+		/*
+		 * We might be called under a transaction (e.g. indirect backref
+		 * resolution) which could deadlock if it triggers memory reclaim.
+		 */
+		nofs_flag = memalloc_nofs_save();
+		ret = btrfs_drew_lock_init(root->snapshot_lock);
+		memalloc_nofs_restore(nofs_flag);
+		if (ret)
+			goto fail;
 	}
-	/*
-	 * We might be called under a transaction (e.g. indirect backref
-	 * resolution) which could deadlock if it triggers memory reclaim
-	 */
-	nofs_flag = memalloc_nofs_save();
-	ret = btrfs_drew_lock_init(root->snapshot_lock);
-	memalloc_nofs_restore(nofs_flag);
-	if (ret)
-		goto fail;
 
 	if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID &&
 	    !btrfs_is_data_reloc_root(root)) {
diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h
index 67c6625797ca..21c41cf8d115 100644
--- a/fs/btrfs/disk-io.h
+++ b/fs/btrfs/disk-io.h
@@ -28,6 +28,8 @@  static inline u64 btrfs_sb_offset(int mirror)
 struct btrfs_new_fs_root_args {
 	/* Preallocated anonymous block device number */
 	dev_t anon_dev;
+	/* Preallocated snapshot lock */
+	struct btrfs_drew_lock *snapshot_lock;
 };
 
 struct btrfs_device;