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

Jeff Layton <[email protected]>
Newsgroups org.kernel.vger.linux-btrfs,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> >   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 <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.