[PATCH v3 0/6] btrfs: handle -ENOMEM errors in some synchronous dirops without aborting

[PATCH v3 0/6] btrfs: handle -ENOMEM errors in some synchronous dirops without aborting

[Jeff Layton]

This version fixes some issues Qu pointed out in review of v2. I did end
up adopting his suggestion to allocate the prealloc container as well,
which makes the API cleaner. Original cover letter follows:

We've had a (relatively small) number of ENOMEM btrfs aborts occur in
synchronous directory morphing codepaths. It's not terribly common, but
there are a few places where an memory allocation failure results in an
abort.

This patchset reworks the code to do the allocations up front, before the
point where we'd have to abort the fs if it fails.

This does not cover all potential cases where this can currently occur:

In particular, a rename that overwrites the target can still abort the
fs if a memory allocation fails. Fixing that is substantially more work,
unfortunately.

This also doesn't cover orphaning a new inode on failure (which can
trigger new memory allocations), so this series is designed to work in
conjunction with with Boris' GFP_NOFAIL series [1].

AFAICT, these are ancient problems, dating back at least to ~2011. I
didn't bother adding Fixes: tags.

AI disclosure: I made heavy use of an LLM in this patchset, from
drafting the initial series to helping test it.

[1] https://lore.kernel.org/linux-btrfs/cover.1784673567.git.boris@bur.io/

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v3:
- btrfs_prealloc_delayed_dir_index() now allocates and returns the
  btrfs_dir_index_prealloc instead of filling in a caller-provided on-stack
  struct, so a NULL pointer means "no prealloc" and callers no longer need
  to use prealloc->item as an is-allocated flag (as suggested by Qu).
- Fix a leak of a caller-supplied prealloc in btrfs_insert_dir_item() when
  btrfs_alloc_path() fails; all error exits now go through a single
  out_free_prealloc label (Qu Wenruo).
- Move the dir index name memcpy into btrfs_prealloc_delayed_dir_index()
  instead of duplicating it at the call sites (Qu Wenruo).
- New patch to use an on-stack path in btrfs_del_orphan_item().
- btrfs_create_new_inode(): persist nlink=0 with btrfs_update_inode() after
  orphaning the new inode. Otherwise orphan cleanup sees nlink > 0, drops
  the orphan item and leaks the inode.
- Pick up Reviewed-by tags from Qu Wenruo.
- Link to v2: https://lore.kernel.org/r/20260804-btrfs-enomem-v2-0-4d923170e8c1@kernel.org

Changes in v2:
- Use an on-stack btrfs_path in btrfs_insert_orphan_item() so the ENOMEM
  recovery does not itself fail on a path allocation.
- Simplify the recovery in btrfs_create_new_inode() to rely on
  btrfs_orphan_add()'s internal abort instead of aborting twice.
- Add ALLOW_ERROR_INJECTION() on btrfs_prealloc_delayed_dir_index() and a
  new fstest (btrfs/351) to exercise the ENOMEM path.
- Link to v1: https://lore.kernel.org/r/20260717-btrfs-enomem-v1-0-cdc9c0e265d0@kernel.org

---
Jeff Layton (6):
      btrfs: use an on-stack path in btrfs_insert_orphan_item()
      btrfs: use an on-stack path in btrfs_del_orphan_item()
      btrfs: split btrfs_insert_delayed_dir_index() into prealloc and commit phases
      btrfs: pre-allocate delayed dir index before btree modification
      btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting
      btrfs: pre-allocate delayed dir index for non-overwrite rename

 fs/btrfs/btrfs_inode.h   |   4 +-
 fs/btrfs/delayed-inode.c | 115 ++++++++++++++++++++++++++++++++++++-----------
 fs/btrfs/delayed-inode.h |  22 ++++++---
 fs/btrfs/dir-item.c      |  42 +++++++++++------
 fs/btrfs/dir-item.h      |   5 ++-
 fs/btrfs/inode.c         |  64 +++++++++++++++++++++-----
 fs/btrfs/orphan.c        |  36 +++++++--------
 fs/btrfs/transaction.c   |   2 +-
 fs/btrfs/tree-log.c      |   4 +-
 9 files changed, 215 insertions(+), 79 deletions(-)
---
base-commit: 76d8783d4c196f4ac990b97ee9f56d2e6797e173
change-id: 20260715-btrfs-enomem-988f2cc36ffd

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH v3 1/6] btrfs: use an on-stack path in btrfs_insert_orphan_item()

[Jeff Layton]

btrfs_insert_orphan_item() allocated a btrfs_path with btrfs_alloc_path()
which returns -ENOMEM on failure. It is called from btrfs_orphan_add(),
so a path allocation failure there turns a recoverable error into a
transaction abort.

btrfs_path is only ~112 bytes, so allocate it on the stack instead.

Assisted-by: LLM
Suggested-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/btrfs/orphan.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/orphan.c b/fs/btrfs/orphan.c
index 9f3ad124104f..72e1adec39d8 100644
--- a/fs/btrfs/orphan.c
+++ b/fs/btrfs/orphan.c
@@ -9,18 +9,17 @@
 int btrfs_insert_orphan_item(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root, u64 offset)
 {
-	BTRFS_PATH_AUTO_FREE(path);
+	struct btrfs_path path = { 0 };
 	struct btrfs_key key;
+	int ret;
 
 	key.objectid = BTRFS_ORPHAN_OBJECTID;
 	key.type = BTRFS_ORPHAN_ITEM_KEY;
 	key.offset = offset;
 
-	path = btrfs_alloc_path();
-	if (!path)
-		return -ENOMEM;
-
-	return btrfs_insert_empty_item(trans, root, path, &key, 0);
+	ret = btrfs_insert_empty_item(trans, root, &path, &key, 0);
+	btrfs_release_path(&path);
+	return ret;
 }
 
 int btrfs_del_orphan_item(struct btrfs_trans_handle *trans,

-- 
2.55.0

[PATCH v3 2/6] btrfs: use an on-stack path in btrfs_del_orphan_item()

[Jeff Layton]

btrfs_del_orphan_item() allocated a btrfs_path with btrfs_alloc_path()
which returns -ENOMEM on failure. It is called from btrfs_orphan_del(),
and btrfs_link() turns any error from it into a transaction abort. So a
path allocation failure there (reachable via linkat() on an O_TMPFILE
under memory pressure) turns a recoverable error into an abort.

btrfs_path is only ~112 bytes, so allocate it on the stack instead.
Unlike the insert case there are multiple exit points after the search,
so release the path via a common out: label.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/btrfs/orphan.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/orphan.c b/fs/btrfs/orphan.c
index 72e1adec39d8..ae1685f26dec 100644
--- a/fs/btrfs/orphan.c
+++ b/fs/btrfs/orphan.c
@@ -25,23 +25,24 @@ int btrfs_insert_orphan_item(struct btrfs_trans_handle *trans,
 int btrfs_del_orphan_item(struct btrfs_trans_handle *trans,
 			  struct btrfs_root *root, u64 offset)
 {
-	BTRFS_PATH_AUTO_FREE(path);
+	struct btrfs_path path = { 0 };
 	struct btrfs_key key;
-	int ret = 0;
+	int ret;
 
 	key.objectid = BTRFS_ORPHAN_OBJECTID;
 	key.type = BTRFS_ORPHAN_ITEM_KEY;
 	key.offset = offset;
 
-	path = btrfs_alloc_path();
-	if (!path)
-		return -ENOMEM;
-
-	ret = btrfs_search_slot(trans, root, &key, path, -1, 1);
+	ret = btrfs_search_slot(trans, root, &key, &path, -1, 1);
 	if (ret < 0)
-		return ret;
-	if (ret)
-		return -ENOENT;
-
-	return btrfs_del_item(trans, root, path);
+		goto out;
+	if (ret) {
+		ret = -ENOENT;
+		goto out;
+	}
+
+	ret = btrfs_del_item(trans, root, &path);
+out:
+	btrfs_release_path(&path);
+	return ret;
 }

-- 
2.55.0

[PATCH v3 3/6] btrfs: split btrfs_insert_delayed_dir_index() into prealloc and commit phases

[Jeff Layton]

Split btrfs_insert_delayed_dir_index() into three functions using a new
btrfs_dir_index_prealloc struct to bundle the pre-allocated resources:

- btrfs_prealloc_delayed_dir_index(): allocates the struct and performs
  the two GFP_NOFS allocations (delayed node + delayed item) that can
  fail with -ENOMEM. Returns the struct, or ERR_PTR on failure.
- btrfs_insert_delayed_dir_index_prealloc(): populates the item data,
  inserts into the rb-tree, and reserves metadata space. Cannot fail
  with -ENOMEM since all allocations were done in the prealloc step.
- btrfs_free_delayed_dir_index_prealloc(): frees pre-allocated
  resources when the caller's btree insertion fails. Tolerates NULL.

The prealloc is returned as a pointer rather than filled into a
caller-provided struct, so that a plain NULL means "no prealloc" and
callers do not need a separate flag to track whether one exists. It is
consumed (and freed) by either the commit or the free helper, so
ownership is unambiguous.

The original btrfs_insert_delayed_dir_index() is refactored into a thin
wrapper that calls the prealloc and commit functions.

This split allows callers to move the fallible memory allocations before
the point of no return (the DIR_ITEM btree insertion), so that -ENOMEM
can be returned cleanly without aborting the transaction.

Assisted-by: LLM
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/btrfs/delayed-inode.c | 128 ++++++++++++++++++++++++++++++++++++++---------
 fs/btrfs/delayed-inode.h |  17 +++++++
 2 files changed, 121 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index db2ffab0941a..af5e6dbf60d3 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -6,6 +6,7 @@
 
 #include <linux/slab.h>
 #include <linux/iversion.h>
+#include <linux/error-injection.h>
 #include "ctree.h"
 #include "fs.h"
 #include "messages.h"
@@ -1469,35 +1470,93 @@ static void btrfs_release_dir_index_item_space(struct btrfs_trans_handle *trans)
 	trans->bytes_reserved -= bytes;
 }
 
-/* Will return 0, -ENOMEM or -EEXIST (index number collision, unexpected). */
-int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,
-				   const char *name, int name_len,
-				   struct btrfs_inode *dir,
-				   const struct btrfs_disk_key *disk_key, u8 flags,
-				   u64 index)
+/*
+ * Pre-allocate a delayed node and delayed item for a dir index insertion and
+ * copy the name into the item. Call this before modifying the btree so that
+ * ENOMEM can be returned before any on-disk state has changed.
+ *
+ * The returned prealloc is consumed by either
+ * btrfs_insert_delayed_dir_index_prealloc() or
+ * btrfs_free_delayed_dir_index_prealloc(); it must not be used afterwards.
+ *
+ * Returns a prealloc on success, ERR_PTR on allocation failure.
+ */
+struct btrfs_dir_index_prealloc *btrfs_prealloc_delayed_dir_index(struct btrfs_inode *dir,
+								  const char *name,
+								  int name_len)
+{
+	struct btrfs_dir_index_prealloc *prealloc;
+	struct btrfs_delayed_node *node;
+	struct btrfs_delayed_item *item;
+
+	prealloc = kzalloc_obj(*prealloc, GFP_NOFS);
+	if (!prealloc)
+		return ERR_PTR(-ENOMEM);
+
+	node = btrfs_get_or_create_delayed_node(dir, &prealloc->tracker);
+	if (IS_ERR(node)) {
+		kfree(prealloc);
+		return ERR_CAST(node);
+	}
+
+	item = btrfs_alloc_delayed_item(sizeof(struct btrfs_dir_item) + name_len,
+					node, BTRFS_DELAYED_INSERTION_ITEM);
+	if (!item) {
+		btrfs_release_delayed_node(node, &prealloc->tracker);
+		kfree(prealloc);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	memcpy(item->data + sizeof(struct btrfs_dir_item), name, name_len);
+
+	prealloc->node = node;
+	prealloc->item = item;
+	return prealloc;
+}
+ALLOW_ERROR_INJECTION(btrfs_prealloc_delayed_dir_index, ERRNO);
+
+/*
+ * Free resources from btrfs_prealloc_delayed_dir_index() when the btree
+ * insertion failed and we will not commit the delayed dir index. Does nothing
+ * if @prealloc is NULL.
+ */
+void btrfs_free_delayed_dir_index_prealloc(struct btrfs_trans_handle *trans,
+					   struct btrfs_dir_index_prealloc *prealloc)
 {
+	if (!prealloc)
+		return;
+
+	btrfs_release_delayed_item(prealloc->item);
+	btrfs_release_dir_index_item_space(trans);
+	btrfs_release_delayed_node(prealloc->node, &prealloc->tracker);
+	kfree(prealloc);
+}
+
+/*
+ * Commit a pre-allocated delayed dir index item. @prealloc must have been
+ * returned by btrfs_prealloc_delayed_dir_index(). This populates the item,
+ * adds it to the delayed node's rb-tree, and reserves metadata space. It
+ * cannot fail with ENOMEM. @prealloc is freed here in all cases.
+ *
+ * Will return 0 or -EEXIST (index number collision, unexpected).
+ */
+int btrfs_insert_delayed_dir_index_prealloc(struct btrfs_trans_handle *trans,
+					    struct btrfs_inode *dir,
+					    struct btrfs_dir_index_prealloc *prealloc,
+					    const struct btrfs_disk_key *disk_key,
+					    u8 flags, u64 index)
+{
+	struct btrfs_delayed_node *delayed_node = prealloc->node;
+	struct btrfs_ref_tracker *tracker = &prealloc->tracker;
+	struct btrfs_delayed_item *delayed_item = prealloc->item;
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	const unsigned int leaf_data_size = BTRFS_LEAF_DATA_SIZE(fs_info);
-	struct btrfs_delayed_node *delayed_node;
-	struct btrfs_ref_tracker delayed_node_tracker;
-	struct btrfs_delayed_item *delayed_item;
+	const int name_len = delayed_item->data_len - sizeof(struct btrfs_dir_item);
 	struct btrfs_dir_item *dir_item;
 	bool reserve_leaf_space;
 	u32 data_len;
 	int ret;
 
-	delayed_node = btrfs_get_or_create_delayed_node(dir, &delayed_node_tracker);
-	if (IS_ERR(delayed_node))
-		return PTR_ERR(delayed_node);
-
-	delayed_item = btrfs_alloc_delayed_item(sizeof(*dir_item) + name_len,
-						delayed_node,
-						BTRFS_DELAYED_INSERTION_ITEM);
-	if (!delayed_item) {
-		ret = -ENOMEM;
-		goto release_node;
-	}
-
 	delayed_item->index = index;
 
 	dir_item = (struct btrfs_dir_item *)delayed_item->data;
@@ -1506,7 +1565,7 @@ int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,
 	btrfs_set_stack_dir_data_len(dir_item, 0);
 	btrfs_set_stack_dir_name_len(dir_item, name_len);
 	btrfs_set_stack_dir_flags(dir_item, flags);
-	memcpy((char *)(dir_item + 1), name, name_len);
+	/* Name was already copied by btrfs_prealloc_delayed_dir_index(). */
 
 	data_len = delayed_item->data_len + sizeof(struct btrfs_item);
 
@@ -1524,7 +1583,9 @@ int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,
 	if (unlikely(ret)) {
 		btrfs_err(trans->fs_info,
 "error adding delayed dir index item, name: %.*s, index: %llu, root: %llu, dir: %llu, dir->index_cnt: %llu, delayed_node->index_cnt: %llu, error: %pe",
-			  name_len, name, index, btrfs_root_id(delayed_node->root),
+			  name_len,
+			  (const char *)(dir_item + 1),
+			  index, btrfs_root_id(delayed_node->root),
 			  delayed_node->inode_id, dir->index_cnt,
 			  delayed_node->index_cnt, ERR_PTR(ret));
 		btrfs_release_delayed_item(delayed_item);
@@ -1562,10 +1623,29 @@ int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,
 	mutex_unlock(&delayed_node->mutex);
 
 release_node:
-	btrfs_release_delayed_node(delayed_node, &delayed_node_tracker);
+	/* Must release the node before freeing @tracker's containing struct. */
+	btrfs_release_delayed_node(delayed_node, tracker);
+	kfree(prealloc);
 	return ret;
 }
 
+/* Will return 0, -ENOMEM or -EEXIST (index number collision, unexpected). */
+int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,
+				   const char *name, int name_len,
+				   struct btrfs_inode *dir,
+				   const struct btrfs_disk_key *disk_key, u8 flags,
+				   u64 index)
+{
+	struct btrfs_dir_index_prealloc *prealloc;
+
+	prealloc = btrfs_prealloc_delayed_dir_index(dir, name, name_len);
+	if (IS_ERR(prealloc))
+		return PTR_ERR(prealloc);
+
+	return btrfs_insert_delayed_dir_index_prealloc(trans, dir, prealloc,
+						       disk_key, flags, index);
+}
+
 static bool btrfs_delete_delayed_insertion_item(struct btrfs_delayed_node *node,
 						u64 index)
 {
diff --git a/fs/btrfs/delayed-inode.h b/fs/btrfs/delayed-inode.h
index fc752863f89b..6d12a145489f 100644
--- a/fs/btrfs/delayed-inode.h
+++ b/fs/btrfs/delayed-inode.h
@@ -121,6 +121,23 @@ int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,
 				   const struct btrfs_disk_key *disk_key, u8 flags,
 				   u64 index);
 
+struct btrfs_dir_index_prealloc {
+	struct btrfs_delayed_node *node;
+	struct btrfs_ref_tracker tracker;
+	struct btrfs_delayed_item *item;
+};
+
+struct btrfs_dir_index_prealloc *btrfs_prealloc_delayed_dir_index(struct btrfs_inode *dir,
+								  const char *name,
+								  int name_len);
+void btrfs_free_delayed_dir_index_prealloc(struct btrfs_trans_handle *trans,
+					   struct btrfs_dir_index_prealloc *prealloc);
+int btrfs_insert_delayed_dir_index_prealloc(struct btrfs_trans_handle *trans,
+					    struct btrfs_inode *dir,
+					    struct btrfs_dir_index_prealloc *prealloc,
+					    const struct btrfs_disk_key *disk_key,
+					    u8 flags, u64 index);
+
 int btrfs_delete_delayed_dir_index(struct btrfs_trans_handle *trans,
 				   struct btrfs_inode *dir, u64 index);
 

-- 
2.55.0

[PATCH v3 4/6] btrfs: pre-allocate delayed dir index before btree modification

[Jeff Layton]

Move the delayed dir index allocation in btrfs_insert_dir_item() before
the insert_with_overflow() call that modifies the btree. Previously, the
allocations happened after the DIR_ITEM was already inserted, meaning an
ENOMEM failure left the btree in a partially-modified state that could
only be resolved by aborting the transaction.

Add an optional caller-provided btrfs_dir_index_prealloc parameter to
btrfs_insert_dir_item(). When non-NULL, ownership of the prealloc
transfers to btrfs_insert_dir_item(). When NULL, it allocates internally.
All existing callers pass NULL to preserve the current behavior.

Since ownership transfers, btrfs_insert_dir_item() must free the prealloc
on every path that does not commit it. Route all such exits (including
the early path allocation failure) through a common out_free_prealloc
label, rather than keying cleanup on need_delayed_index.

Remove the btrfs_insert_delayed_dir_index() wrapper, as there are no
more callers.

Assisted-by: LLM
Suggested-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/btrfs/delayed-inode.c | 21 ++-------------------
 fs/btrfs/delayed-inode.h |  5 -----
 fs/btrfs/dir-item.c      | 42 ++++++++++++++++++++++++++++--------------
 fs/btrfs/dir-item.h      |  5 +++--
 fs/btrfs/inode.c         |  2 +-
 fs/btrfs/transaction.c   |  2 +-
 6 files changed, 35 insertions(+), 42 deletions(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index af5e6dbf60d3..bd603525c133 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -687,7 +687,7 @@ static int btrfs_insert_delayed_item(struct btrfs_trans_handle *trans,
 	/*
 	 * For delayed items to insert, we track reserved metadata bytes based
 	 * on the number of leaves that we will use.
-	 * See btrfs_insert_delayed_dir_index() and
+	 * See btrfs_insert_delayed_dir_index_prealloc() and
 	 * btrfs_delayed_item_reserve_metadata()).
 	 */
 	ASSERT(first_item->bytes_reserved == 0);
@@ -1629,23 +1629,6 @@ int btrfs_insert_delayed_dir_index_prealloc(struct btrfs_trans_handle *trans,
 	return ret;
 }
 
-/* Will return 0, -ENOMEM or -EEXIST (index number collision, unexpected). */
-int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,
-				   const char *name, int name_len,
-				   struct btrfs_inode *dir,
-				   const struct btrfs_disk_key *disk_key, u8 flags,
-				   u64 index)
-{
-	struct btrfs_dir_index_prealloc *prealloc;
-
-	prealloc = btrfs_prealloc_delayed_dir_index(dir, name, name_len);
-	if (IS_ERR(prealloc))
-		return PTR_ERR(prealloc);
-
-	return btrfs_insert_delayed_dir_index_prealloc(trans, dir, prealloc,
-						       disk_key, flags, index);
-}
-
 static bool btrfs_delete_delayed_insertion_item(struct btrfs_delayed_node *node,
 						u64 index)
 {
@@ -1661,7 +1644,7 @@ static bool btrfs_delete_delayed_insertion_item(struct btrfs_delayed_node *node,
 	/*
 	 * For delayed items to insert, we track reserved metadata bytes based
 	 * on the number of leaves that we will use.
-	 * See btrfs_insert_delayed_dir_index() and
+	 * See btrfs_insert_delayed_dir_index_prealloc() and
 	 * btrfs_delayed_item_reserve_metadata()).
 	 */
 	ASSERT(item->bytes_reserved == 0);
diff --git a/fs/btrfs/delayed-inode.h b/fs/btrfs/delayed-inode.h
index 6d12a145489f..57ba96cfaf9c 100644
--- a/fs/btrfs/delayed-inode.h
+++ b/fs/btrfs/delayed-inode.h
@@ -115,11 +115,6 @@ struct btrfs_delayed_item {
 };
 
 void btrfs_init_delayed_root(struct btrfs_delayed_root *delayed_root);
-int btrfs_insert_delayed_dir_index(struct btrfs_trans_handle *trans,
-				   const char *name, int name_len,
-				   struct btrfs_inode *dir,
-				   const struct btrfs_disk_key *disk_key, u8 flags,
-				   u64 index);
 
 struct btrfs_dir_index_prealloc {
 	struct btrfs_delayed_node *node;
diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index 84f1c64423d3..30ddafaf8d3d 100644
--- a/fs/btrfs/dir-item.c
+++ b/fs/btrfs/dir-item.c
@@ -106,8 +106,11 @@ int btrfs_insert_xattr_item(struct btrfs_trans_handle *trans,
  * Will return 0 or -ENOMEM
  */
 int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
-			  const struct fscrypt_str *name, struct btrfs_inode *dir,
-			  const struct btrfs_key *location, u8 type, u64 index)
+			  const struct fscrypt_str *name,
+			  struct btrfs_inode *dir,
+			  const struct btrfs_key *location, u8 type,
+			  u64 index,
+			  struct btrfs_dir_index_prealloc *prealloc)
 {
 	int ret = 0;
 	int ret2 = 0;
@@ -119,17 +122,28 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 	struct btrfs_key key;
 	struct btrfs_disk_key disk_key;
 	u32 data_size;
+	const bool need_delayed_index = (root != root->fs_info->tree_root);
 
 	key.objectid = btrfs_ino(dir);
 	key.type = BTRFS_DIR_ITEM_KEY;
 	key.offset = btrfs_name_hash(name->name, name->len);
 
 	path = btrfs_alloc_path();
-	if (!path)
-		return -ENOMEM;
+	if (!path) {
+		ret = -ENOMEM;
+		goto out_free_prealloc;
+	}
 
 	btrfs_cpu_key_to_disk(&disk_key, location);
 
+	/* Pre-allocate the delayed dir index before modifying the btree. */
+	if (need_delayed_index && !prealloc) {
+		prealloc = btrfs_prealloc_delayed_dir_index(dir, name->name,
+							    name->len);
+		if (IS_ERR(prealloc))
+			return PTR_ERR(prealloc);
+	}
+
 	data_size = sizeof(*dir_item) + name->len;
 	dir_item = insert_with_overflow(trans, root, path, &key, data_size,
 					name->name, name->len);
@@ -137,7 +151,7 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 		ret = PTR_ERR(dir_item);
 		if (ret == -EEXIST)
 			goto second_insert;
-		goto out_free;
+		goto out_free_prealloc;
 	}
 
 	if (IS_ENCRYPTED(&dir->vfs_inode))
@@ -154,21 +168,21 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 	write_extent_buffer(leaf, name->name, name_ptr, name->len);
 
 second_insert:
-	/* FIXME, use some real flag for selecting the extra index */
-	if (root == root->fs_info->tree_root) {
+	if (!need_delayed_index) {
 		ret = 0;
-		goto out_free;
+		goto out_free_prealloc;
 	}
 	btrfs_release_path(path);
 
-	ret2 = btrfs_insert_delayed_dir_index(trans, name->name, name->len, dir,
-					      &disk_key, type, index);
-out_free:
+	ret2 = btrfs_insert_delayed_dir_index_prealloc(trans, dir, prealloc,
+						       &disk_key, type, index);
 	if (ret)
 		return ret;
-	if (ret2)
-		return ret2;
-	return 0;
+	return ret2;
+
+out_free_prealloc:
+	btrfs_free_delayed_dir_index_prealloc(trans, prealloc);
+	return ret;
 }
 
 static struct btrfs_dir_item *btrfs_lookup_match_dir(
diff --git a/fs/btrfs/dir-item.h b/fs/btrfs/dir-item.h
index e52174a8baf9..d7a7d0b66f37 100644
--- a/fs/btrfs/dir-item.h
+++ b/fs/btrfs/dir-item.h
@@ -16,9 +16,11 @@ struct btrfs_trans_handle;
 
 int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir_ino,
 			  const struct fscrypt_str *name);
+struct btrfs_dir_index_prealloc;
 int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 			  const struct fscrypt_str *name, struct btrfs_inode *dir,
-			  const struct btrfs_key *location, u8 type, u64 index);
+			  const struct btrfs_key *location, u8 type, u64 index,
+			  struct btrfs_dir_index_prealloc *prealloc);
 struct btrfs_dir_item *btrfs_lookup_dir_item(struct btrfs_trans_handle *trans,
 					     struct btrfs_root *root,
 					     struct btrfs_path *path, u64 dir,
@@ -53,5 +55,4 @@ static inline u64 btrfs_name_hash(const char *name, int len)
 {
        return crc32c((u32)~1, name, len);
 }
-
 #endif
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3c10a0ef0002..3a2dca093c7d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6924,7 +6924,7 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
 		return ret;
 
 	ret = btrfs_insert_dir_item(trans, name, parent_inode, &key,
-				    btrfs_inode_type(inode), index);
+				    btrfs_inode_type(inode), index, NULL);
 	if (ret == -EEXIST || ret == -EOVERFLOW)
 		goto fail_dir_item;
 	else if (unlikely(ret)) {
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index c641099d66e2..6fdfea5d35af 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1882,7 +1882,7 @@ static noinline int create_pending_snapshot(struct btrfs_trans_handle *trans,
 
 	ret = btrfs_insert_dir_item(trans, &fname.disk_name,
 				    parent_inode, &key, BTRFS_FT_DIR,
-				    index);
+				    index, NULL);
 	if (unlikely(ret)) {
 		btrfs_abort_transaction(trans, ret);
 		goto fail;

-- 
2.55.0

[PATCH v3 5/6] btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting

[Jeff Layton]

Now that btrfs_insert_dir_item() returns -ENOMEM before modifying the
btree (thanks to delayed dir index pre-allocation), callers can handle
ENOMEM gracefully instead of aborting the transaction.

- btrfs_add_link(): add -ENOMEM to the recoverable errors alongside
  -EEXIST and -EOVERFLOW.
- btrfs_create_new_inode(): on -ENOMEM from btrfs_add_link(), orphan the
  newly-created inode instead of aborting. The inode item was already
  written with nlink 1, and discard_new_inode() marks it bad so eviction
  won't delete it. So clear_nlink() alone is not enough: persist nlink 0
  via btrfs_update_inode(), otherwise orphan cleanup would see nlink > 0,
  drop the orphan item, and leak the inode. Fall back to aborting only if
  that update also fails.

This turns a filesystem-killing abort into a graceful -ENOMEM return for
create(), mkdir(), mknod(), symlink(), and link() under memory pressure.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/btrfs/inode.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 3a2dca093c7d..5b79910d72f5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6863,7 +6863,27 @@ int btrfs_create_new_inode(struct btrfs_trans_handle *trans,
 	} else {
 		ret = btrfs_add_link(trans, BTRFS_I(dir), BTRFS_I(inode), name,
 				     false, BTRFS_I(inode)->dir_index);
-		if (unlikely(ret)) {
+		if (ret == -ENOMEM) {
+			/*
+			 * Orphan the new inode instead of aborting. The inode
+			 * item was already written with nlink 1, and discard's
+			 * eviction won't delete a bad inode, so nlink 0 must be
+			 * persisted here or orphan cleanup would see nlink > 0,
+			 * drop the orphan item, and leak the inode.
+			 */
+			clear_nlink(inode);
+			/* btrfs_orphan_add() aborts the transaction on failure. */
+			ret = btrfs_orphan_add(trans, BTRFS_I(inode));
+			if (ret)
+				goto discard;
+			ret = btrfs_update_inode(trans, BTRFS_I(inode));
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
+				goto discard;
+			}
+			ret = -ENOMEM;
+			goto discard;
+		} else if (unlikely(ret)) {
 			btrfs_abort_transaction(trans, ret);
 			goto discard;
 		}
@@ -6925,7 +6945,7 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
 
 	ret = btrfs_insert_dir_item(trans, name, parent_inode, &key,
 				    btrfs_inode_type(inode), index, NULL);
-	if (ret == -EEXIST || ret == -EOVERFLOW)
+	if (ret == -EEXIST || ret == -EOVERFLOW || ret == -ENOMEM)
 		goto fail_dir_item;
 	else if (unlikely(ret)) {
 		btrfs_abort_transaction(trans, ret);

-- 
2.55.0

[PATCH v3 6/6] btrfs: pre-allocate delayed dir index for non-overwrite rename

[Jeff Layton]

For rename() without an overwrite target, pre-allocate the delayed
dir index before any btree modifications so that ENOMEM can be returned
before the source is unlinked from the old directory.

Add a prealloc parameter to btrfs_add_link() that allows callers to
pass pre-allocated delayed dir index resources. When provided,
btrfs_add_link() takes ownership: it either passes the prealloc to
btrfs_insert_dir_item() (which commits or frees it), or frees it
on early error. All existing callers pass NULL to preserve the current
behavior.

In btrfs_rename(), when new_inode is NULL (no overwrite), call
btrfs_prealloc_delayed_dir_index() before the first btree modification
and pass the result through to btrfs_add_link(). If the prealloc fails,
-ENOMEM is returned before any btree state has changed. The local
prealloc pointer is cleared once ownership passes to btrfs_add_link(),
so the out_fail path only frees one we still own.

For overwrite rename (new_inode != NULL), the transaction still aborts
on ENOMEM since earlier unlink operations have already made irreversible
btree modifications.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/btrfs/btrfs_inode.h |  4 +++-
 fs/btrfs/inode.c       | 40 ++++++++++++++++++++++++++++++++--------
 fs/btrfs/tree-log.c    |  4 ++--
 3 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index 1082fa92c145..d4280f152027 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -525,9 +525,11 @@ int btrfs_set_inode_index(struct btrfs_inode *dir, u64 *index);
 int btrfs_unlink_inode(struct btrfs_trans_handle *trans,
 		       struct btrfs_inode *dir, struct btrfs_inode *inode,
 		       const struct fscrypt_str *name);
+struct btrfs_dir_index_prealloc;
 int btrfs_add_link(struct btrfs_trans_handle *trans,
 		   struct btrfs_inode *parent_inode, struct btrfs_inode *inode,
-		   const struct fscrypt_str *name, bool add_backref, u64 index);
+		   const struct fscrypt_str *name, bool add_backref, u64 index,
+		   struct btrfs_dir_index_prealloc *prealloc);
 int btrfs_delete_subvolume(struct btrfs_inode *dir, struct dentry *dentry);
 int btrfs_truncate_block(struct btrfs_inode *inode, u64 offset, u64 start, u64 end);
 
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 5b79910d72f5..fd6d481f4d12 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6862,7 +6862,7 @@ int btrfs_create_new_inode(struct btrfs_trans_handle *trans,
 		}
 	} else {
 		ret = btrfs_add_link(trans, BTRFS_I(dir), BTRFS_I(inode), name,
-				     false, BTRFS_I(inode)->dir_index);
+				     false, BTRFS_I(inode)->dir_index, NULL);
 		if (ret == -ENOMEM) {
 			/*
 			 * Orphan the new inode instead of aborting. The inode
@@ -6914,7 +6914,8 @@ int btrfs_create_new_inode(struct btrfs_trans_handle *trans,
  */
 int btrfs_add_link(struct btrfs_trans_handle *trans,
 		   struct btrfs_inode *parent_inode, struct btrfs_inode *inode,
-		   const struct fscrypt_str *name, bool add_backref, u64 index)
+		   const struct fscrypt_str *name, bool add_backref, u64 index,
+		   struct btrfs_dir_index_prealloc *prealloc)
 {
 	int ret = 0;
 	struct btrfs_key key;
@@ -6940,11 +6941,13 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
 	}
 
 	/* Nothing to clean up yet */
-	if (ret)
+	if (ret) {
+		btrfs_free_delayed_dir_index_prealloc(trans, prealloc);
 		return ret;
+	}
 
 	ret = btrfs_insert_dir_item(trans, name, parent_inode, &key,
-				    btrfs_inode_type(inode), index, NULL);
+				    btrfs_inode_type(inode), index, prealloc);
 	if (ret == -EEXIST || ret == -EOVERFLOW || ret == -ENOMEM)
 		goto fail_dir_item;
 	else if (unlikely(ret)) {
@@ -7098,7 +7101,7 @@ static int btrfs_link(struct dentry *old_dentry, struct inode *dir,
 	inode_set_ctime_current(inode);
 
 	ret = btrfs_add_link(trans, BTRFS_I(dir), BTRFS_I(inode),
-			     &fname.disk_name, true, index);
+			     &fname.disk_name, true, index, NULL);
 	if (ret)
 		goto fail;
 
@@ -8512,14 +8515,14 @@ static int btrfs_rename_exchange(struct inode *old_dir,
 	}
 
 	ret = btrfs_add_link(trans, BTRFS_I(new_dir), BTRFS_I(old_inode),
-			     new_name, false, old_idx);
+			     new_name, false, old_idx, NULL);
 	if (unlikely(ret)) {
 		btrfs_abort_transaction(trans, ret);
 		goto out_fail;
 	}
 
 	ret = btrfs_add_link(trans, BTRFS_I(old_dir), BTRFS_I(new_inode),
-			     old_name, false, new_idx);
+			     old_name, false, new_idx, NULL);
 	if (unlikely(ret)) {
 		btrfs_abort_transaction(trans, ret);
 		goto out_fail;
@@ -8592,6 +8595,7 @@ static int btrfs_rename(struct mnt_idmap *idmap,
 	struct inode *new_inode = d_inode(new_dentry);
 	struct inode *old_inode = d_inode(old_dentry);
 	struct btrfs_rename_ctx rename_ctx;
+	struct btrfs_dir_index_prealloc *prealloc = NULL;
 	u64 index = 0;
 	int ret;
 	int ret2;
@@ -8715,6 +8719,24 @@ static int btrfs_rename(struct mnt_idmap *idmap,
 	if (ret)
 		goto out_fail;
 
+	/*
+	 * When not overwriting an existing entry, pre-allocate the delayed
+	 * dir index now so that ENOMEM is returned before any btree
+	 * modifications. For the overwrite case, too many btree changes
+	 * have already happened by the time btrfs_add_link() is called.
+	 */
+	if (!new_inode) {
+		prealloc = btrfs_prealloc_delayed_dir_index(
+				BTRFS_I(new_dir),
+				new_fname.disk_name.name,
+				new_fname.disk_name.len);
+		if (IS_ERR(prealloc)) {
+			ret = PTR_ERR(prealloc);
+			prealloc = NULL;
+			goto out_fail;
+		}
+	}
+
 	BTRFS_I(old_inode)->dir_index = 0ULL;
 	if (unlikely(old_ino == BTRFS_FIRST_FREE_OBJECTID)) {
 		/* force full log commit if subvolume involved. */
@@ -8810,7 +8832,8 @@ static int btrfs_rename(struct mnt_idmap *idmap,
 	}
 
 	ret = btrfs_add_link(trans, BTRFS_I(new_dir), BTRFS_I(old_inode),
-			     &new_fname.disk_name, false, index);
+			     &new_fname.disk_name, false, index, prealloc);
+	prealloc = NULL;
 	if (unlikely(ret)) {
 		btrfs_abort_transaction(trans, ret);
 		goto out_fail;
@@ -8835,6 +8858,7 @@ static int btrfs_rename(struct mnt_idmap *idmap,
 		}
 	}
 out_fail:
+	btrfs_free_delayed_dir_index_prealloc(trans, prealloc);
 	if (logs_pinned) {
 		btrfs_end_log_trans(root);
 		btrfs_end_log_trans(dest);
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index 7ba7b6098aa5..a043611f82e1 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -1683,7 +1683,7 @@ static noinline int add_inode_ref(struct walk_control *wc)
 			}
 
 			/* insert our name */
-			ret = btrfs_add_link(trans, dir, inode, &name, false, ref_index);
+			ret = btrfs_add_link(trans, dir, inode, &name, false, ref_index, NULL);
 			if (ret) {
 				btrfs_abort_log_replay(wc, ret,
 "failed to add link for inode %llu in dir %llu ref_index %llu name %.*s root %llu",
@@ -2031,7 +2031,7 @@ static noinline int insert_one_name(struct btrfs_trans_handle *trans,
 		return PTR_ERR(dir);
 	}
 
-	ret = btrfs_add_link(trans, dir, inode, name, true, index);
+	ret = btrfs_add_link(trans, dir, inode, name, true, index, NULL);
 
 	/* FIXME, put inode into FIXUP list */
 

-- 
2.55.0

Re: [PATCH v3 0/6] btrfs: handle -ENOMEM errors in some synchronous dirops without aborting

[Qu Wenruo]

在 2026/8/12 03:44, Jeff Layton 写道:
> This version fixes some issues Qu pointed out in review of v2. I did end
> up adopting his suggestion to allocate the prealloc container as well,
> which makes the API cleaner. Original cover letter follows:
> 
> We've had a (relatively small) number of ENOMEM btrfs aborts occur in
> synchronous directory morphing codepaths. It's not terribly common, but
> there are a few places where an memory allocation failure results in an
> abort.
> 
> This patchset reworks the code to do the allocations up front, before the
> point where we'd have to abort the fs if it fails.
> 
> This does not cover all potential cases where this can currently occur:
> 
> In particular, a rename that overwrites the target can still abort the
> fs if a memory allocation fails. Fixing that is substantially more work,
> unfortunately.
> 
> This also doesn't cover orphaning a new inode on failure (which can
> trigger new memory allocations), so this series is designed to work in
> conjunction with with Boris' GFP_NOFAIL series [1].
> 
> AFAICT, these are ancient problems, dating back at least to ~2011. I
> didn't bother adding Fixes: tags.
> 
> AI disclosure: I made heavy use of an LLM in this patchset, from
> drafting the initial series to helping test it.
> 
> [1] https://lore.kernel.org/linux-btrfs/cover.1784673567.git.boris@bur.io/
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
> Changes in v3:
> - btrfs_prealloc_delayed_dir_index() now allocates and returns the
>    btrfs_dir_index_prealloc instead of filling in a caller-provided on-stack
>    struct, so a NULL pointer means "no prealloc" and callers no longer need
>    to use prealloc->item as an is-allocated flag (as suggested by Qu).
> - Fix a leak of a caller-supplied prealloc in btrfs_insert_dir_item() when
>    btrfs_alloc_path() fails; all error exits now go through a single
>    out_free_prealloc label (Qu Wenruo).
> - Move the dir index name memcpy into btrfs_prealloc_delayed_dir_index()
>    instead of duplicating it at the call sites (Qu Wenruo).
> - New patch to use an on-stack path in btrfs_del_orphan_item().
> - btrfs_create_new_inode(): persist nlink=0 with btrfs_update_inode() after
>    orphaning the new inode. Otherwise orphan cleanup sees nlink > 0, drops
>    the orphan item and leaks the inode.
> - Pick up Reviewed-by tags from Qu Wenruo.
> - Link to v2: https://lore.kernel.org/r/20260804-btrfs-enomem-v2-0-4d923170e8c1@kernel.org
> 
> Changes in v2:
> - Use an on-stack btrfs_path in btrfs_insert_orphan_item() so the ENOMEM
>    recovery does not itself fail on a path allocation.
> - Simplify the recovery in btrfs_create_new_inode() to rely on
>    btrfs_orphan_add()'s internal abort instead of aborting twice.
> - Add ALLOW_ERROR_INJECTION() on btrfs_prealloc_delayed_dir_index() and a
>    new fstest (btrfs/351) to exercise the ENOMEM path.
> - Link to v1: https://lore.kernel.org/r/20260717-btrfs-enomem-v1-0-cdc9c0e265d0@kernel.org
> 
> ---
> Jeff Layton (6):
>        btrfs: use an on-stack path in btrfs_insert_orphan_item()
>        btrfs: use an on-stack path in btrfs_del_orphan_item()
>        btrfs: split btrfs_insert_delayed_dir_index() into prealloc and commit phases
>        btrfs: pre-allocate delayed dir index before btree modification
>        btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting
>        btrfs: pre-allocate delayed dir index for non-overwrite rename
> 
>   fs/btrfs/btrfs_inode.h   |   4 +-
>   fs/btrfs/delayed-inode.c | 115 ++++++++++++++++++++++++++++++++++++-----------
>   fs/btrfs/delayed-inode.h |  22 ++++++---
>   fs/btrfs/dir-item.c      |  42 +++++++++++------
>   fs/btrfs/dir-item.h      |   5 ++-
>   fs/btrfs/inode.c         |  64 +++++++++++++++++++++-----
>   fs/btrfs/orphan.c        |  36 +++++++--------
>   fs/btrfs/transaction.c   |   2 +-
>   fs/btrfs/tree-log.c      |   4 +-
>   9 files changed, 215 insertions(+), 79 deletions(-)
> ---
> base-commit: 76d8783d4c196f4ac990b97ee9f56d2e6797e173
> change-id: 20260715-btrfs-enomem-988f2cc36ffd
> 
> Best regards,

Re: [PATCH v3 1/6] btrfs: use an on-stack path in btrfs_insert_orphan_item()

[David Sterba]

On Tue, Aug 11, 2026 at 02:14:54PM -0400, Jeff Layton wrote:
> btrfs_insert_orphan_item() allocated a btrfs_path with btrfs_alloc_path()
> which returns -ENOMEM on failure. It is called from btrfs_orphan_add(),
> so a path allocation failure there turns a recoverable error into a
> transaction abort.
> 
> btrfs_path is only ~112 bytes, so allocate it on the stack instead.

112 is too much for on-stack, we've avoided that for btrfs_path in
particular, except some justified cases. This means in general the
beginning of call stack like ioctl, syscall handler and such. Otherwise
we assume there are other layers in the IO stack, like block device
drivers (DM), NFS, encoding layers or networking (iscsi), and obviously
the lowest level device drivers.

The trade off with possible allocation failure vs stack consumption
needs to be argued in the changelog, "is just 112" is not sufficient.

Getting back the consumed stack space is painful, we've been reducing
unneeded or redundant parameters of functions for years. The gains are
like -8 bytes here and -8 bytes there, allocation of +112 wipes that out.

If the place of allocation is critical we can consider that but we have
too many of them, anywhere during the transaction commit path or
irreversible metadata changes. Possibly using __GFP_HIGH could work, but
I haven't explored that.

Qu added the patches to for-next but I had no chance to look closely at
this patchset yet and am hesitant to leave it like that.

Re: [PATCH v3 1/6] btrfs: use an on-stack path in btrfs_insert_orphan_item()

[Jeff Layton]

On Thu, 2026-08-20 at 14:04 +0200, David Sterba wrote:
> On Tue, Aug 11, 2026 at 02:14:54PM -0400, Jeff Layton wrote:
> > btrfs_insert_orphan_item() allocated a btrfs_path with btrfs_alloc_path()
> > which returns -ENOMEM on failure. It is called from btrfs_orphan_add(),
> > so a path allocation failure there turns a recoverable error into a
> > transaction abort.
> > 
> > btrfs_path is only ~112 bytes, so allocate it on the stack instead.
> 
> 112 is too much for on-stack, we've avoided that for btrfs_path in
> particular, except some justified cases. This means in general the
> beginning of call stack like ioctl, syscall handler and such. Otherwise
> we assume there are other layers in the IO stack, like block device
> drivers (DM), NFS, encoding layers or networking (iscsi), and obviously
> the lowest level device drivers.
> 
> The trade off with possible allocation failure vs stack consumption
> needs to be argued in the changelog, "is just 112" is not sufficient.
>
> Getting back the consumed stack space is painful, we've been reducing
> unneeded or redundant parameters of functions for years. The gains are
> like -8 bytes here and -8 bytes there, allocation of +112 wipes that out.
> 
> If the place of allocation is critical we can consider that but we have
> too many of them, anywhere during the transaction commit path or
> irreversible metadata changes. Possibly using __GFP_HIGH could work, but
> I haven't explored that.
> 
> Qu added the patches to for-next but I had no chance to look closely at
> this patchset yet and am hesitant to leave it like that.

Fair critique. These days I think we're mostly ~16k stacks, but it can
certainly get deep given the right layering.

In this case, failing this allocation can lead to aborting the fs, so
it seemed justified. This is a GFP_NOFS allocation too, so it can
easily fail, even though it's small.

Qu suggested this approach in an earlier review pass, so I'd be
interested to hear his take on this too.

Thanks for taking a look!
-- 
Jeff Layton <jlayton@kernel.org>