The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@kernel.org>
To: Qu Wenruo <wqu@suse.com>, Chris Mason <clm@fb.com>,
	David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
	 kernel-team@fb.com
Subject: Re: [PATCH v2 2/5] btrfs: split btrfs_insert_delayed_dir_index() into prealloc and commit phases
Date: Wed, 05 Aug 2026 07:54:20 -0400	[thread overview]
Message-ID: <8ebbafc475baff8aedf5e2b2718db867063fab07.camel@kernel.org> (raw)
In-Reply-To: <8b16c994-b322-42c9-b134-47dca5f2682e@suse.com>

On Wed, 2026-08-05 at 08:44 +0930, 'Qu Wenruo' via Kernel Team wrote:
> 
> 在 2026/8/5 01:14, 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 | 111 +++++++++++++++++++++++++++++++++++++----------
> >   fs/btrfs/delayed-inode.h |  17 ++++++++
> >   2 files changed, 104 insertions(+), 24 deletions(-)
> > 
> > diff --git a/fs/btrfs/delayed-inode.c b/fs/btrfs/delayed-inode.c
> > index db2ffab0941a..95d2dca80444 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,74 @@ 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)
> 
> We can directly return a btrfs_dir_index_prealloc pointer, which reduce 
> the parameter list.
> 

...but then we'd have to allocate it separately. It's currently
allocated on the stack by the caller. I think we ought to keep it this
way, but I'm willing to be convinced otherwise.

> Otherwise looks good to me.
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>
> 

-- 
Jeff Layton <jlayton@kernel.org>

  reply	other threads:[~2026-08-05 11:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 15:44 [PATCH v2 0/5] btrfs: handle -ENOMEM errors in some synchronous dirops without aborting Jeff Layton
2026-08-04 15:44 ` [PATCH v2 1/5] btrfs: use an on-stack path in btrfs_insert_orphan_item() Jeff Layton
2026-08-04 23:18   ` Qu Wenruo
2026-08-04 15:44 ` [PATCH v2 2/5] btrfs: split btrfs_insert_delayed_dir_index() into prealloc and commit phases Jeff Layton
2026-08-04 23:14   ` Qu Wenruo
2026-08-05 11:54     ` Jeff Layton [this message]
2026-08-05 22:34       ` Qu Wenruo
2026-08-05 22:43         ` Jeff Layton
2026-08-04 15:44 ` [PATCH v2 3/5] btrfs: pre-allocate delayed dir index before btree modification Jeff Layton
2026-08-04 23:51   ` Qu Wenruo
2026-08-05 12:09     ` Jeff Layton
2026-08-04 15:44 ` [PATCH v2 4/5] btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting Jeff Layton
2026-08-04 15:44 ` [PATCH v2 5/5] btrfs: pre-allocate delayed dir index for non-overwrite rename Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8ebbafc475baff8aedf5e2b2718db867063fab07.camel@kernel.org \
    --to=jlayton@kernel.org \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox