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

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

[Jeff Layton]

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, unfortunately
substantially more work, but this should help improve things.

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

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Jeff Layton (4):
      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 | 109 ++++++++++++++++++++++++++++++++++++-----------
 fs/btrfs/delayed-inode.h |  17 ++++++++
 fs/btrfs/dir-item.c      |  30 ++++++++++---
 fs/btrfs/dir-item.h      |   5 ++-
 fs/btrfs/inode.c         |  60 +++++++++++++++++++++-----
 fs/btrfs/transaction.c   |   2 +-
 fs/btrfs/tree-log.c      |   4 +-
 8 files changed, 185 insertions(+), 46 deletions(-)
---
base-commit: af5e34a41cd607c00ef752e00331736570992354
change-id: 20260715-btrfs-enomem-988f2cc36ffd

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

[PATCH 1/4] 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(): performs the two GFP_NOFS
  allocations (delayed node + delayed item) that can fail with -ENOMEM.
- 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.

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
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/btrfs/delayed-inode.c | 109 ++++++++++++++++++++++++++++++++++++-----------
 fs/btrfs/delayed-inode.h |  17 ++++++++
 2 files changed, 102 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
index 09795439b9fb..9d7343934eea 100644
--- a/fs/btrfs/delayed-inode.c
+++ b/fs/btrfs/delayed-inode.c
@@ -1469,35 +1469,73 @@ 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.
+ * Call this before modifying the btree so that ENOMEM can be returned
+ * before any on-disk state has changed.
+ *
+ * Returns 0 on success, -ENOMEM on allocation failure.
+ */
+int 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;
+
+	node = btrfs_get_or_create_delayed_node(dir, &prealloc->tracker);
+	if (IS_ERR(node))
+		return PTR_ERR(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);
+		return -ENOMEM;
+	}
+
+	prealloc->node = node;
+	prealloc->item = item;
+	return 0;
+}
+
+/*
+ * Free resources from btrfs_prealloc_delayed_dir_index() when the btree
+ * insertion failed and we will not commit the delayed dir index.
+ */
+void btrfs_free_delayed_dir_index_prealloc(struct btrfs_trans_handle *trans,
+					   struct btrfs_dir_index_prealloc *prealloc)
 {
+	btrfs_release_delayed_item(prealloc->item);
+	btrfs_release_dir_index_item_space(trans);
+	btrfs_release_delayed_node(prealloc->node, &prealloc->tracker);
+}
+
+/*
+ * Commit a pre-allocated delayed dir index item. The delayed node and item
+ * 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.
+ *
+ * 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 +1544,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 into delayed_item->data by the caller. */
 
 	data_len = delayed_item->data_len + sizeof(struct btrfs_item);
 
@@ -1524,7 +1562,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: %d",
-			  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, ret);
 		btrfs_release_delayed_item(delayed_item);
@@ -1562,10 +1602,31 @@ 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);
+	btrfs_release_delayed_node(delayed_node, tracker);
 	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;
+	int ret;
+
+	ret = btrfs_prealloc_delayed_dir_index(dir, name, name_len, &prealloc);
+	if (ret)
+		return ret;
+
+	memcpy(prealloc.item->data + sizeof(struct btrfs_dir_item), name,
+	       name_len);
+
+	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..e310a257c9a6 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;
+};
+
+int btrfs_prealloc_delayed_dir_index(struct btrfs_inode *dir,
+				     const char *name, int name_len,
+				     struct btrfs_dir_index_prealloc *prealloc);
+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 2/4] 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.

With the pre-allocation, if the GFP_NOFS allocations fail, -ENOMEM is
returned before any on-disk state has changed. The caller can then
handle the error gracefully. If the btree insertion itself fails, the
pre-allocated resources are freed.

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.

Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/btrfs/dir-item.c    | 30 ++++++++++++++++++++++++------
 fs/btrfs/dir-item.h    |  5 +++--
 fs/btrfs/inode.c       |  2 +-
 fs/btrfs/transaction.c |  2 +-
 4 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/dir-item.c b/fs/btrfs/dir-item.c
index 84f1c64423d3..1b956df2c571 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,6 +122,8 @@ 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);
+	struct btrfs_dir_index_prealloc local_prealloc;
 
 	key.objectid = btrfs_ino(dir);
 	key.type = BTRFS_DIR_ITEM_KEY;
@@ -130,6 +135,18 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 
 	btrfs_cpu_key_to_disk(&disk_key, location);
 
+	/* Pre-allocate the delayed dir index before modifying the btree. */
+	if (need_delayed_index && !prealloc) {
+		ret = btrfs_prealloc_delayed_dir_index(dir, name->name,
+						       name->len,
+						       &local_prealloc);
+		if (ret)
+			return ret;
+		memcpy(local_prealloc.item->data + sizeof(struct btrfs_dir_item),
+		       name->name, name->len);
+		prealloc = &local_prealloc;
+	}
+
 	data_size = sizeof(*dir_item) + name->len;
 	dir_item = insert_with_overflow(trans, root, path, &key, data_size,
 					name->name, name->len);
@@ -137,6 +154,8 @@ int btrfs_insert_dir_item(struct btrfs_trans_handle *trans,
 		ret = PTR_ERR(dir_item);
 		if (ret == -EEXIST)
 			goto second_insert;
+		if (need_delayed_index)
+			btrfs_free_delayed_dir_index_prealloc(trans, prealloc);
 		goto out_free;
 	}
 
@@ -154,15 +173,14 @@ 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;
 	}
 	btrfs_release_path(path);
 
-	ret2 = btrfs_insert_delayed_dir_index(trans, name->name, name->len, dir,
-					      &disk_key, type, index);
+	ret2 = btrfs_insert_delayed_dir_index_prealloc(trans, dir, prealloc,
+						       &disk_key, type, index);
 out_free:
 	if (ret)
 		return ret;
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 272598f6ae77..b7b4e6177135 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6737,7 +6737,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 8f9419728100..3a2d4520c79a 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1886,7 +1886,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 3/4] 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.

In btrfs_add_link(), add -ENOMEM to the set of recoverable errors
alongside -EEXIST and -EOVERFLOW. The fail_dir_item cleanup path
unwinds the inode_ref/root_ref and returns the error to userspace.

In btrfs_create_new_inode(), when btrfs_add_link() fails with -ENOMEM,
convert the newly-created inode into an orphan instead of aborting.
This is done by clearing nlink and adding an orphan item, which ensures
btrfs_evict_inode() will delete the INODE_ITEM and INODE_REF, and
crash-recovery will clean it up via orphan processing. If
btrfs_orphan_add() itself fails, we fall back to aborting.

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

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

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index b7b4e6177135..4d9947ae08f7 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6676,7 +6676,20 @@ 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) {
+			/*
+			 * The ENOMEM came before the DIR_ITEM was inserted,
+			 * so the btree has our INODE_ITEM + INODE_REF but no
+			 * directory entry. Convert this into an orphan so
+			 * eviction (or crash-recovery) cleans up the inode.
+			 */
+			clear_nlink(inode);
+			ret = btrfs_orphan_add(trans, BTRFS_I(inode));
+			if (unlikely(ret))
+				btrfs_abort_transaction(trans, ret);
+			ret = -ENOMEM;
+			goto discard;
+		} else if (unlikely(ret)) {
 			btrfs_abort_transaction(trans, ret);
 			goto discard;
 		}
@@ -6738,7 +6751,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 4/4] 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.

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       | 43 +++++++++++++++++++++++++++++++++++--------
 fs/btrfs/tree-log.c    |  4 ++--
 3 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h
index d5d81f9546c3..176648f6e7b6 100644
--- a/fs/btrfs/btrfs_inode.h
+++ b/fs/btrfs/btrfs_inode.h
@@ -523,9 +523,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 4d9947ae08f7..9a717d21f713 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -6675,7 +6675,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) {
 			/*
 			 * The ENOMEM came before the DIR_ITEM was inserted,
@@ -6720,7 +6720,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;
@@ -6746,11 +6747,14 @@ int btrfs_add_link(struct btrfs_trans_handle *trans,
 	}
 
 	/* Nothing to clean up yet */
-	if (ret)
+	if (ret) {
+		if (prealloc)
+			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)) {
@@ -6904,7 +6908,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;
 
@@ -8311,14 +8315,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;
@@ -8391,6 +8395,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 = { };
 	u64 index = 0;
 	int ret;
 	int ret2;
@@ -8514,6 +8519,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) {
+		ret = btrfs_prealloc_delayed_dir_index(
+				BTRFS_I(new_dir),
+				new_fname.disk_name.name,
+				new_fname.disk_name.len,
+				&prealloc);
+		if (ret)
+			goto out_fail;
+		memcpy(prealloc.item->data + sizeof(struct btrfs_dir_item),
+		       new_fname.disk_name.name, new_fname.disk_name.len);
+	}
+
 	BTRFS_I(old_inode)->dir_index = 0ULL;
 	if (unlikely(old_ino == BTRFS_FIRST_FREE_OBJECTID)) {
 		/* force full log commit if subvolume involved. */
@@ -8609,7 +8632,9 @@ 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.item ? &prealloc : NULL);
+	prealloc.item = NULL;
 	if (unlikely(ret)) {
 		btrfs_abort_transaction(trans, ret);
 		goto out_fail;
@@ -8634,6 +8659,8 @@ static int btrfs_rename(struct mnt_idmap *idmap,
 		}
 	}
 out_fail:
+	if (prealloc.item)
+		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 875e4ddc68ea..86a924d7cd43 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -1700,7 +1700,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",
@@ -2048,7 +2048,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 3/4] btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting

[Boris Burkov]

On Fri, Jul 17, 2026 at 12:52:38PM -0400, Jeff Layton wrote:
> 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.
> 
> In btrfs_add_link(), add -ENOMEM to the set of recoverable errors
> alongside -EEXIST and -EOVERFLOW. The fail_dir_item cleanup path
> unwinds the inode_ref/root_ref and returns the error to userspace.
> 
> In btrfs_create_new_inode(), when btrfs_add_link() fails with -ENOMEM,
> convert the newly-created inode into an orphan instead of aborting.
> This is done by clearing nlink and adding an orphan item, which ensures
> btrfs_evict_inode() will delete the INODE_ITEM and INODE_REF, and
> crash-recovery will clean it up via orphan processing. If
> btrfs_orphan_add() itself fails, we fall back to aborting.
> 
> This turns a filesystem-killing transaction abort into a graceful
> -ENOMEM return to userspace for create(), mkdir(), mknod(), symlink(),
> and link() operations under memory pressure.
> 
> Assisted-by: LLM
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/btrfs/inode.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index b7b4e6177135..4d9947ae08f7 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -6676,7 +6676,20 @@ 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) {
> +			/*
> +			 * The ENOMEM came before the DIR_ITEM was inserted,
> +			 * so the btree has our INODE_ITEM + INODE_REF but no
> +			 * directory entry. Convert this into an orphan so
> +			 * eviction (or crash-recovery) cleans up the inode.
> +			 */
> +			clear_nlink(inode);
> +			ret = btrfs_orphan_add(trans, BTRFS_I(inode));
> +			if (unlikely(ret))
> +				btrfs_abort_transaction(trans, ret);

I feel like the crux of this series to me is whether you have practical
conditions where the allocation of the delayed_node is failing, but the
allocations involved in btrfs_orphan_add() succeed. It allocates a
btrfs_path and has to walk the btree which might have to read the node
at every level which might need to allocate 16k extent buffers and
extent buffer objects and xarray storage for each one. For size
reference, on my build (maybe debug..?) a delayed_node is 552 bytes,
while a btrfs_path is 112 and an extent_buffer is 432. So they are
pretty similar in size (not to mention the 16k of node file backed
memory we are sort of likely to have to allocate if we are under
reclaim)

Were you able to reproduce this issue and help in practice or is this a
theoretical / structural improvement?

With that said, all the prealloc wiring looks good to me in general, and
it seems to be a pretty clean win for the "name exists" case in the next
patch.

Thanks,
Boris

> +			ret = -ENOMEM;
> +			goto discard;
> +		} else if (unlikely(ret)) {
>  			btrfs_abort_transaction(trans, ret);
>  			goto discard;
>  		}
> @@ -6738,7 +6751,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
> 

Re: [PATCH 3/4] btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting

[Jeff Layton]

On Fri, 2026-07-17 at 13:18 -0700, Boris Burkov wrote:
> On Fri, Jul 17, 2026 at 12:52:38PM -0400, Jeff Layton wrote:
> > 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.
> > 
> > In btrfs_add_link(), add -ENOMEM to the set of recoverable errors
> > alongside -EEXIST and -EOVERFLOW. The fail_dir_item cleanup path
> > unwinds the inode_ref/root_ref and returns the error to userspace.
> > 
> > In btrfs_create_new_inode(), when btrfs_add_link() fails with -ENOMEM,
> > convert the newly-created inode into an orphan instead of aborting.
> > This is done by clearing nlink and adding an orphan item, which ensures
> > btrfs_evict_inode() will delete the INODE_ITEM and INODE_REF, and
> > crash-recovery will clean it up via orphan processing. If
> > btrfs_orphan_add() itself fails, we fall back to aborting.
> > 
> > This turns a filesystem-killing transaction abort into a graceful
> > -ENOMEM return to userspace for create(), mkdir(), mknod(), symlink(),
> > and link() operations under memory pressure.
> > 
> > Assisted-by: LLM
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  fs/btrfs/inode.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index b7b4e6177135..4d9947ae08f7 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -6676,7 +6676,20 @@ 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) {
> > +			/*
> > +			 * The ENOMEM came before the DIR_ITEM was inserted,
> > +			 * so the btree has our INODE_ITEM + INODE_REF but no
> > +			 * directory entry. Convert this into an orphan so
> > +			 * eviction (or crash-recovery) cleans up the inode.
> > +			 */
> > +			clear_nlink(inode);
> > +			ret = btrfs_orphan_add(trans, BTRFS_I(inode));
> > +			if (unlikely(ret))
> > +				btrfs_abort_transaction(trans, ret);
> 
> I feel like the crux of this series to me is whether you have practical
> conditions where the allocation of the delayed_node is failing, but the
> allocations involved in btrfs_orphan_add() succeed. It allocates a
> btrfs_path and has to walk the btree which might have to read the node
> at every level which might need to allocate 16k extent buffers and
> extent buffer objects and xarray storage for each one. For size
> reference, on my build (maybe debug..?) a delayed_node is 552 bytes,
> while a btrfs_path is 112 and an extent_buffer is 432. So they are
> pretty similar in size (not to mention the 16k of node file backed
> memory we are sort of likely to have to allocate if we are under
> reclaim)
> 
> Were you able to reproduce this issue and help in practice or is this a
> theoretical / structural improvement?
> 

I didn't really try to reproduce this in earnest. We only see it in our
fleet under heavy memory pressure, and even then at such low frequency,
I doubt our chances of hitting this on anything other than a huge set
of machines.

So, theoretical / structural, but we have record of filesystem aborts
where the stack indicates that this would have prevented it. Userland
would have gotten an -ENOMEM back but the fs wouldn't have aborted.

I see that there are some ALLOW_ERROR_INJECTION() calls in btrfs. We
could wire some of these functions up with that, which would make this
easier to test. I'll look into that in the meantime.

> With that said, all the prealloc wiring looks good to me in general, and
> it seems to be a pretty clean win for the "name exists" case in the next
> patch.
> 
> 

Thanks for the review!

> > +			ret = -ENOMEM;
> > +			goto discard;
> > +		} else if (unlikely(ret)) {
> >  			btrfs_abort_transaction(trans, ret);
> >  			goto discard;
> >  		}
> > @@ -6738,7 +6751,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
> > 

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 3/4] btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting

[Boris Burkov]

On Fri, Jul 17, 2026 at 06:40:42PM -0400, Jeff Layton wrote:
> On Fri, 2026-07-17 at 13:18 -0700, Boris Burkov wrote:
> > On Fri, Jul 17, 2026 at 12:52:38PM -0400, Jeff Layton wrote:
> > > 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.
> > > 
> > > In btrfs_add_link(), add -ENOMEM to the set of recoverable errors
> > > alongside -EEXIST and -EOVERFLOW. The fail_dir_item cleanup path
> > > unwinds the inode_ref/root_ref and returns the error to userspace.
> > > 
> > > In btrfs_create_new_inode(), when btrfs_add_link() fails with -ENOMEM,
> > > convert the newly-created inode into an orphan instead of aborting.
> > > This is done by clearing nlink and adding an orphan item, which ensures
> > > btrfs_evict_inode() will delete the INODE_ITEM and INODE_REF, and
> > > crash-recovery will clean it up via orphan processing. If
> > > btrfs_orphan_add() itself fails, we fall back to aborting.
> > > 
> > > This turns a filesystem-killing transaction abort into a graceful
> > > -ENOMEM return to userspace for create(), mkdir(), mknod(), symlink(),
> > > and link() operations under memory pressure.
> > > 
> > > Assisted-by: LLM
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  fs/btrfs/inode.c | 17 +++++++++++++++--
> > >  1 file changed, 15 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > > index b7b4e6177135..4d9947ae08f7 100644
> > > --- a/fs/btrfs/inode.c
> > > +++ b/fs/btrfs/inode.c
> > > @@ -6676,7 +6676,20 @@ 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) {
> > > +			/*
> > > +			 * The ENOMEM came before the DIR_ITEM was inserted,
> > > +			 * so the btree has our INODE_ITEM + INODE_REF but no
> > > +			 * directory entry. Convert this into an orphan so
> > > +			 * eviction (or crash-recovery) cleans up the inode.
> > > +			 */
> > > +			clear_nlink(inode);
> > > +			ret = btrfs_orphan_add(trans, BTRFS_I(inode));
> > > +			if (unlikely(ret))
> > > +				btrfs_abort_transaction(trans, ret);
> > 
> > I feel like the crux of this series to me is whether you have practical
> > conditions where the allocation of the delayed_node is failing, but the
> > allocations involved in btrfs_orphan_add() succeed. It allocates a
> > btrfs_path and has to walk the btree which might have to read the node
> > at every level which might need to allocate 16k extent buffers and
> > extent buffer objects and xarray storage for each one. For size
> > reference, on my build (maybe debug..?) a delayed_node is 552 bytes,
> > while a btrfs_path is 112 and an extent_buffer is 432. So they are
> > pretty similar in size (not to mention the 16k of node file backed
> > memory we are sort of likely to have to allocate if we are under
> > reclaim)
> > 
> > Were you able to reproduce this issue and help in practice or is this a
> > theoretical / structural improvement?
> > 
> 
> I didn't really try to reproduce this in earnest. We only see it in our
> fleet under heavy memory pressure, and even then at such low frequency,
> I doubt our chances of hitting this on anything other than a huge set
> of machines.
> 
> So, theoretical / structural, but we have record of filesystem aborts
> where the stack indicates that this would have prevented it. Userland
> would have gotten an -ENOMEM back but the fs wouldn't have aborted.
> 

My concern is not that we don't hit ENOMEM in btrfs_add_link(), since
like you said we can observe that in abort logs. I am worried that even
if we try to handle it gracefully, we will just ENOMEM in
btrfs_orphan_add() and abort anyway. That is why I was wanting to see
some more concrete evidence this actually helps to make it worth the
complexity.

> I see that there are some ALLOW_ERROR_INJECTION() calls in btrfs. We
> could wire some of these functions up with that, which would make this
> easier to test. I'll look into that in the meantime.
> 
> > With that said, all the prealloc wiring looks good to me in general, and
> > it seems to be a pretty clean win for the "name exists" case in the next
> > patch.
> > 
> > 
> 
> Thanks for the review!
> 
> > > +			ret = -ENOMEM;
> > > +			goto discard;
> > > +		} else if (unlikely(ret)) {
> > >  			btrfs_abort_transaction(trans, ret);
> > >  			goto discard;
> > >  		}
> > > @@ -6738,7 +6751,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
> > > 
> 
> -- 
> Jeff Layton <jlayton@kernel.org>

Re: [PATCH 3/4] btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting

[Jeff Layton]

On Fri, 2026-07-17 at 16:04 -0700, Boris Burkov wrote:
> On Fri, Jul 17, 2026 at 06:40:42PM -0400, Jeff Layton wrote:
> > On Fri, 2026-07-17 at 13:18 -0700, Boris Burkov wrote:
> > > On Fri, Jul 17, 2026 at 12:52:38PM -0400, Jeff Layton wrote:
> > > > 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.
> > > > 
> > > > In btrfs_add_link(), add -ENOMEM to the set of recoverable errors
> > > > alongside -EEXIST and -EOVERFLOW. The fail_dir_item cleanup path
> > > > unwinds the inode_ref/root_ref and returns the error to userspace.
> > > > 
> > > > In btrfs_create_new_inode(), when btrfs_add_link() fails with -ENOMEM,
> > > > convert the newly-created inode into an orphan instead of aborting.
> > > > This is done by clearing nlink and adding an orphan item, which ensures
> > > > btrfs_evict_inode() will delete the INODE_ITEM and INODE_REF, and
> > > > crash-recovery will clean it up via orphan processing. If
> > > > btrfs_orphan_add() itself fails, we fall back to aborting.
> > > > 
> > > > This turns a filesystem-killing transaction abort into a graceful
> > > > -ENOMEM return to userspace for create(), mkdir(), mknod(), symlink(),
> > > > and link() operations under memory pressure.
> > > > 
> > > > Assisted-by: LLM
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > >  fs/btrfs/inode.c | 17 +++++++++++++++--
> > > >  1 file changed, 15 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > > > index b7b4e6177135..4d9947ae08f7 100644
> > > > --- a/fs/btrfs/inode.c
> > > > +++ b/fs/btrfs/inode.c
> > > > @@ -6676,7 +6676,20 @@ 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) {
> > > > +			/*
> > > > +			 * The ENOMEM came before the DIR_ITEM was inserted,
> > > > +			 * so the btree has our INODE_ITEM + INODE_REF but no
> > > > +			 * directory entry. Convert this into an orphan so
> > > > +			 * eviction (or crash-recovery) cleans up the inode.
> > > > +			 */
> > > > +			clear_nlink(inode);
> > > > +			ret = btrfs_orphan_add(trans, BTRFS_I(inode));
> > > > +			if (unlikely(ret))
> > > > +				btrfs_abort_transaction(trans, ret);
> > > 
> > > I feel like the crux of this series to me is whether you have practical
> > > conditions where the allocation of the delayed_node is failing, but the
> > > allocations involved in btrfs_orphan_add() succeed. It allocates a
> > > btrfs_path and has to walk the btree which might have to read the node
> > > at every level which might need to allocate 16k extent buffers and
> > > extent buffer objects and xarray storage for each one. For size
> > > reference, on my build (maybe debug..?) a delayed_node is 552 bytes,
> > > while a btrfs_path is 112 and an extent_buffer is 432. So they are
> > > pretty similar in size (not to mention the 16k of node file backed
> > > memory we are sort of likely to have to allocate if we are under
> > > reclaim)
> > > 
> > > Were you able to reproduce this issue and help in practice or is this a
> > > theoretical / structural improvement?
> > > 
> > 
> > I didn't really try to reproduce this in earnest. We only see it in our
> > fleet under heavy memory pressure, and even then at such low frequency,
> > I doubt our chances of hitting this on anything other than a huge set
> > of machines.
> > 
> > So, theoretical / structural, but we have record of filesystem aborts
> > where the stack indicates that this would have prevented it. Userland
> > would have gotten an -ENOMEM back but the fs wouldn't have aborted.
> > 
> 
> My concern is not that we don't hit ENOMEM in btrfs_add_link(), since
> like you said we can observe that in abort logs. I am worried that even
> if we try to handle it gracefully, we will just ENOMEM in
> btrfs_orphan_add() and abort anyway. That is why I was wanting to see
> some more concrete evidence this actually helps to make it worth the
> complexity.
> 

In the case where we handle this gracefully, we won't hit that because
it will have returned -ENOMEM before that point. But, you do have a
good point that we could allocate these objects successfully, and then
hit an error in btrfs_orphan_add() anyway.

One thought: It looks like the main allocation in that codepath is
btrfs_alloc_path()? We could consider preallocating that too -- maybe
stash it in a new pointer in btrfs_trans_handle?

Thanks! This is good food for thought.

> > I see that there are some ALLOW_ERROR_INJECTION() calls in btrfs. We
> > could wire some of these functions up with that, which would make this
> > easier to test. I'll look into that in the meantime.
> > 
> > > With that said, all the prealloc wiring looks good to me in general, and
> > > it seems to be a pretty clean win for the "name exists" case in the next
> > > patch.
> > > 
> > > 
> > 
> > Thanks for the review!
> > 
> > > > +			ret = -ENOMEM;
> > > > +			goto discard;
> > > > +		} else if (unlikely(ret)) {
> > > >  			btrfs_abort_transaction(trans, ret);
> > > >  			goto discard;
> > > >  		}
> > > > @@ -6738,7 +6751,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
> > > > 
> > 
> > -- 
> > Jeff Layton <jlayton@kernel.org>

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 3/4] btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting

[Boris Burkov]

On Fri, Jul 17, 2026 at 07:55:35PM -0400, Jeff Layton wrote:
> On Fri, 2026-07-17 at 16:04 -0700, Boris Burkov wrote:
> > On Fri, Jul 17, 2026 at 06:40:42PM -0400, Jeff Layton wrote:
> > > On Fri, 2026-07-17 at 13:18 -0700, Boris Burkov wrote:
> > > > On Fri, Jul 17, 2026 at 12:52:38PM -0400, Jeff Layton wrote:
> > > > > 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.
> > > > > 
> > > > > In btrfs_add_link(), add -ENOMEM to the set of recoverable errors
> > > > > alongside -EEXIST and -EOVERFLOW. The fail_dir_item cleanup path
> > > > > unwinds the inode_ref/root_ref and returns the error to userspace.
> > > > > 
> > > > > In btrfs_create_new_inode(), when btrfs_add_link() fails with -ENOMEM,
> > > > > convert the newly-created inode into an orphan instead of aborting.
> > > > > This is done by clearing nlink and adding an orphan item, which ensures > > > > > btrfs_evict_inode() will delete the INODE_ITEM and INODE_REF, and
> > > > > crash-recovery will clean it up via orphan processing. If
> > > > > btrfs_orphan_add() itself fails, we fall back to aborting.
> > > > > 
> > > > > This turns a filesystem-killing transaction abort into a graceful
> > > > > -ENOMEM return to userspace for create(), mkdir(), mknod(), symlink(),
> > > > > and link() operations under memory pressure.
> > > > > 
> > > > > Assisted-by: LLM
> > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > > ---
> > > > >  fs/btrfs/inode.c | 17 +++++++++++++++--
> > > > >  1 file changed, 15 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > > > > index b7b4e6177135..4d9947ae08f7 100644
> > > > > --- a/fs/btrfs/inode.c
> > > > > +++ b/fs/btrfs/inode.c
> > > > > @@ -6676,7 +6676,20 @@ 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) {

So we are now handling ENOMEM from btrfs_add_link()

> > > > > +			/*
> > > > > +			 * The ENOMEM came before the DIR_ITEM was inserted,
> > > > > +			 * so the btree has our INODE_ITEM + INODE_REF but no
> > > > > +			 * directory entry. Convert this into an orphan so
> > > > > +			 * eviction (or crash-recovery) cleans up the inode.
> > > > > +			 */
> > > > > +			clear_nlink(inode);
> > > > > +			ret = btrfs_orphan_add(trans, BTRFS_I(inode));

And when we get ENOMEM, we call btrfs_orphan_add() isntead of
unilaterally aborting. I contend that this call is now likely to ENOMEM

> > > > > +			if (unlikely(ret))
> > > > > +				btrfs_abort_transaction(trans, ret);

And abort here.

> > > > 
> > > > I feel like the crux of this series to me is whether you have practical
> > > > conditions where the allocation of the delayed_node is failing, but the
> > > > allocations involved in btrfs_orphan_add() succeed. It allocates a
> > > > btrfs_path and has to walk the btree which might have to read the node
> > > > at every level which might need to allocate 16k extent buffers and
> > > > extent buffer objects and xarray storage for each one. For size
> > > > reference, on my build (maybe debug..?) a delayed_node is 552 bytes,
> > > > while a btrfs_path is 112 and an extent_buffer is 432. So they are
> > > > pretty similar in size (not to mention the 16k of node file backed
> > > > memory we are sort of likely to have to allocate if we are under
> > > > reclaim)
> > > > 
> > > > Were you able to reproduce this issue and help in practice or is this a
> > > > theoretical / structural improvement?
> > > > 
> > > 
> > > I didn't really try to reproduce this in earnest. We only see it in our
> > > fleet under heavy memory pressure, and even then at such low frequency,
> > > I doubt our chances of hitting this on anything other than a huge set
> > > of machines.
> > > 
> > > So, theoretical / structural, but we have record of filesystem aborts
> > > where the stack indicates that this would have prevented it. Userland
> > > would have gotten an -ENOMEM back but the fs wouldn't have aborted.
> > > 
> > 
> > My concern is not that we don't hit ENOMEM in btrfs_add_link(), since
> > like you said we can observe that in abort logs. I am worried that even
> > if we try to handle it gracefully, we will just ENOMEM in
> > btrfs_orphan_add() and abort anyway. That is why I was wanting to see
> > some more concrete evidence this actually helps to make it worth the
> > complexity.
> > 
> 
> In the case where we handle this gracefully, we won't hit that because
> it will have returned -ENOMEM before that point. But, you do have a
> good point that we could allocate these objects successfully, and then
> hit an error in btrfs_orphan_add() anyway.

I am still a bit confused. I made comments inline closer to the the code.
Sorry if I am being dense or missing the point!

> 
> One thought: It looks like the main allocation in that codepath is
> btrfs_alloc_path()? We could consider preallocating that too -- maybe
> stash it in a new pointer in btrfs_trans_handle?
> 
> Thanks! This is good food for thought.

Take a peek at https://lore.kernel.org/linux-btrfs/cover.1782249000.git.boris@bur.io/

which I need to RESEND :)

Thanks,
Boris

> 
> > > I see that there are some ALLOW_ERROR_INJECTION() calls in btrfs. We
> > > could wire some of these functions up with that, which would make this
> > > easier to test. I'll look into that in the meantime.
> > > 
> > > > With that said, all the prealloc wiring looks good to me in general, and
> > > > it seems to be a pretty clean win for the "name exists" case in the next
> > > > patch.
> > > > 
> > > > 
> > > 
> > > Thanks for the review!
> > > 
> > > > > +			ret = -ENOMEM;
> > > > > +			goto discard;
> > > > > +		} else if (unlikely(ret)) {
> > > > >  			btrfs_abort_transaction(trans, ret);
> > > > >  			goto discard;
> > > > >  		}
> > > > > @@ -6738,7 +6751,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
> > > > > 
> > > 
> > > -- 
> > > Jeff Layton <jlayton@kernel.org>
> 
> -- 
> Jeff Layton <jlayton@kernel.org>

Re: [PATCH 3/4] btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting

[Qu Wenruo]

在 2026/7/18 09:25, Jeff Layton 写道:
> On Fri, 2026-07-17 at 16:04 -0700, Boris Burkov wrote:
>> On Fri, Jul 17, 2026 at 06:40:42PM -0400, Jeff Layton wrote:
>>> On Fri, 2026-07-17 at 13:18 -0700, Boris Burkov wrote:
>>>> On Fri, Jul 17, 2026 at 12:52:38PM -0400, Jeff Layton wrote:
>>>>> 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.
>>>>>
>>>>> In btrfs_add_link(), add -ENOMEM to the set of recoverable errors
>>>>> alongside -EEXIST and -EOVERFLOW. The fail_dir_item cleanup path
>>>>> unwinds the inode_ref/root_ref and returns the error to userspace.
>>>>>
>>>>> In btrfs_create_new_inode(), when btrfs_add_link() fails with -ENOMEM,
>>>>> convert the newly-created inode into an orphan instead of aborting.
>>>>> This is done by clearing nlink and adding an orphan item, which ensures
>>>>> btrfs_evict_inode() will delete the INODE_ITEM and INODE_REF, and
>>>>> crash-recovery will clean it up via orphan processing. If
>>>>> btrfs_orphan_add() itself fails, we fall back to aborting.
>>>>>
>>>>> This turns a filesystem-killing transaction abort into a graceful
>>>>> -ENOMEM return to userspace for create(), mkdir(), mknod(), symlink(),
>>>>> and link() operations under memory pressure.
>>>>>
>>>>> Assisted-by: LLM
>>>>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>>>>> ---
>>>>>   fs/btrfs/inode.c | 17 +++++++++++++++--
>>>>>   1 file changed, 15 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>>>>> index b7b4e6177135..4d9947ae08f7 100644
>>>>> --- a/fs/btrfs/inode.c
>>>>> +++ b/fs/btrfs/inode.c
>>>>> @@ -6676,7 +6676,20 @@ 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) {
>>>>> +			/*
>>>>> +			 * The ENOMEM came before the DIR_ITEM was inserted,
>>>>> +			 * so the btree has our INODE_ITEM + INODE_REF but no
>>>>> +			 * directory entry. Convert this into an orphan so
>>>>> +			 * eviction (or crash-recovery) cleans up the inode.
>>>>> +			 */
>>>>> +			clear_nlink(inode);
>>>>> +			ret = btrfs_orphan_add(trans, BTRFS_I(inode));
>>>>> +			if (unlikely(ret))
>>>>> +				btrfs_abort_transaction(trans, ret);
>>>>
>>>> I feel like the crux of this series to me is whether you have practical
>>>> conditions where the allocation of the delayed_node is failing, but the
>>>> allocations involved in btrfs_orphan_add() succeed. It allocates a
>>>> btrfs_path and has to walk the btree which might have to read the node
>>>> at every level which might need to allocate 16k extent buffers and
>>>> extent buffer objects and xarray storage for each one. For size
>>>> reference, on my build (maybe debug..?) a delayed_node is 552 bytes,
>>>> while a btrfs_path is 112 and an extent_buffer is 432. So they are
>>>> pretty similar in size (not to mention the 16k of node file backed
>>>> memory we are sort of likely to have to allocate if we are under
>>>> reclaim)
>>>>
>>>> Were you able to reproduce this issue and help in practice or is this a
>>>> theoretical / structural improvement?
>>>>
>>>
>>> I didn't really try to reproduce this in earnest. We only see it in our
>>> fleet under heavy memory pressure, and even then at such low frequency,
>>> I doubt our chances of hitting this on anything other than a huge set
>>> of machines.
>>>
>>> So, theoretical / structural, but we have record of filesystem aborts
>>> where the stack indicates that this would have prevented it. Userland
>>> would have gotten an -ENOMEM back but the fs wouldn't have aborted.
>>>
>>
>> My concern is not that we don't hit ENOMEM in btrfs_add_link(), since
>> like you said we can observe that in abort logs. I am worried that even
>> if we try to handle it gracefully, we will just ENOMEM in
>> btrfs_orphan_add() and abort anyway. That is why I was wanting to see
>> some more concrete evidence this actually helps to make it worth the
>> complexity.
>>
> 
> In the case where we handle this gracefully, we won't hit that because
> it will have returned -ENOMEM before that point. But, you do have a
> good point that we could allocate these objects successfully, and then
> hit an error in btrfs_orphan_add() anyway.
> 
> One thought: It looks like the main allocation in that codepath is
> btrfs_alloc_path()? We could consider preallocating that too -- maybe
> stash it in a new pointer in btrfs_trans_handle?

For btrfs_alloc_path(), you can just use on-stack memory for that.

That's the practice we utilize for btrfs-progs, and it's still 
acceptable for kernel, since that structure is only 112 bytes.

We should not switch all btrfs_path to on-stack ones, but for critical 
ones that memory allocation can lead to trans abort, I'd say it's 
definitely worth the extra on-stack memory usage.

Thanks,
Qu