From: robbieko <robbieko@synology.com>
To: linux-btrfs@vger.kernel.org
Cc: Robbie Ko <robbieko@synology.com>
Subject: [PATCH v2 3/3] btrfs: add snapshot_lock to new_fs_root_args
Date: Wed, 14 Dec 2022 10:11:25 +0800 [thread overview]
Message-ID: <20221214021125.28289-4-robbieko@synology.com> (raw)
In-Reply-To: <20221214021125.28289-1-robbieko@synology.com>
From: Robbie Ko <robbieko@synology.com>
[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 <robbieko@synology.com>
---
fs/btrfs/disk-io.c | 44 +++++++++++++++++++++++++++++++-------------
fs/btrfs/disk-io.h | 2 ++
2 files changed, 33 insertions(+), 13 deletions(-)
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;
--
2.17.1
next prev parent reply other threads:[~2022-12-14 2:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-14 2:11 [PATCH v2 0/3] btrfs: fix unexpected -ENOMEM with percpu_counter_init when create snapshot robbieko
2022-12-14 2:11 ` [PATCH v2 1/3] btrfs: refactor anon_dev with new_fs_root_args for create subvolume/snapshot robbieko
2022-12-14 2:11 ` [PATCH v2 2/3] btrfs: change snapshot_lock to dynamic pointer robbieko
2022-12-14 2:11 ` robbieko [this message]
2022-12-14 16:59 ` [PATCH v2 0/3] btrfs: fix unexpected -ENOMEM with percpu_counter_init when create snapshot Josef Bacik
2022-12-14 18:07 ` David Sterba
2022-12-19 6:54 ` robbieko
2023-01-10 15:08 ` David Sterba
2023-03-09 18:47 ` David Sterba
2023-03-10 1:07 ` robbieko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221214021125.28289-4-robbieko@synology.com \
--to=robbieko@synology.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).