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@xxxxxxxxxx>
> ---
>  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
>