Re: [PATCH v2 3/5] btrfs: pre-allocate delayed dir index before btree modification

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2026-08-05 at 09:21 +0930, Qu Wenruo wrote:
> 
> 在 2026/8/5 01:14, 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.
> > 
> > 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      | 30 ++++++++++++++++++++++++------
> >   fs/btrfs/dir-item.h      |  5 +++--
> >   fs/btrfs/inode.c         |  2 +-
> >   fs/btrfs/transaction.c   |  2 +-
> >   6 files changed, 29 insertions(+), 36 deletions(-)
> > 
> > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> > index 95d2dca80444..d9de7f269874 100644
> > --- a/fs/btrfs/delayed-inode.c
> > +++ b/fs/btrfs/delayed-inode.c
> > @@ -1608,27 +1608,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;
> > -	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 e310a257c9a6..878d70aee2f9 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..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) {
> 
> This is exposed by sashiko.
> 
> If we have @prealloc passed in, and before we even hit 
> insert_with_overflow(), the previous btrfs_alloc_path() failed, we 
> return -ENOMEM directly, leaking the @prealloc.
> 
> 

Good catch. It looks like sashiko also flagged some other similar
issues nearby too that we might as well fix while we're in here. I'll
plan to spin up a v3 and resend the series. Thanks very much for the
review so far!

-- 
Jeff Layton <jlayton@kernel.org>