mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: Jeff Layton <jlayton@kernel.org>, 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 v3 0/6] btrfs: handle -ENOMEM errors in some synchronous dirops without aborting
Date: Thu, 13 Aug 2026 08:52:26 +0930	[thread overview]
Message-ID: <1943a03e-5569-4a8b-b008-14211517c805@suse.com> (raw)
In-Reply-To: <20260811-btrfs-enomem-v3-0-46a993fc3fe5@kernel.org>



在 2026/8/12 03:44, Jeff Layton 写道:
> This version fixes some issues Qu pointed out in review of v2. I did end
> up adopting his suggestion to allocate the prealloc container as well,
> which makes the API cleaner. Original cover letter follows:
> 
> 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 substantially more work,
> unfortunately.
> 
> This also doesn't cover orphaning a new inode on failure (which can
> trigger new memory allocations), so this series is designed to work in
> conjunction with with Boris' GFP_NOFAIL series [1].
> 
> AFAICT, these are ancient problems, dating back at least to ~2011. I
> didn't bother adding Fixes: tags.
> 
> AI disclosure: I made heavy use of an LLM in this patchset, from
> drafting the initial series to helping test it.
> 
> [1] https://lore.kernel.org/linux-btrfs/cover.1784673567.git.boris@bur.io/
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
> Changes in v3:
> - btrfs_prealloc_delayed_dir_index() now allocates and returns the
>    btrfs_dir_index_prealloc instead of filling in a caller-provided on-stack
>    struct, so a NULL pointer means "no prealloc" and callers no longer need
>    to use prealloc->item as an is-allocated flag (as suggested by Qu).
> - Fix a leak of a caller-supplied prealloc in btrfs_insert_dir_item() when
>    btrfs_alloc_path() fails; all error exits now go through a single
>    out_free_prealloc label (Qu Wenruo).
> - Move the dir index name memcpy into btrfs_prealloc_delayed_dir_index()
>    instead of duplicating it at the call sites (Qu Wenruo).
> - New patch to use an on-stack path in btrfs_del_orphan_item().
> - btrfs_create_new_inode(): persist nlink=0 with btrfs_update_inode() after
>    orphaning the new inode. Otherwise orphan cleanup sees nlink > 0, drops
>    the orphan item and leaks the inode.
> - Pick up Reviewed-by tags from Qu Wenruo.
> - Link to v2: https://lore.kernel.org/r/20260804-btrfs-enomem-v2-0-4d923170e8c1@kernel.org
> 
> Changes in v2:
> - Use an on-stack btrfs_path in btrfs_insert_orphan_item() so the ENOMEM
>    recovery does not itself fail on a path allocation.
> - Simplify the recovery in btrfs_create_new_inode() to rely on
>    btrfs_orphan_add()'s internal abort instead of aborting twice.
> - Add ALLOW_ERROR_INJECTION() on btrfs_prealloc_delayed_dir_index() and a
>    new fstest (btrfs/351) to exercise the ENOMEM path.
> - Link to v1: https://lore.kernel.org/r/20260717-btrfs-enomem-v1-0-cdc9c0e265d0@kernel.org
> 
> ---
> Jeff Layton (6):
>        btrfs: use an on-stack path in btrfs_insert_orphan_item()
>        btrfs: use an on-stack path in btrfs_del_orphan_item()
>        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 | 115 ++++++++++++++++++++++++++++++++++++-----------
>   fs/btrfs/delayed-inode.h |  22 ++++++---
>   fs/btrfs/dir-item.c      |  42 +++++++++++------
>   fs/btrfs/dir-item.h      |   5 ++-
>   fs/btrfs/inode.c         |  64 +++++++++++++++++++++-----
>   fs/btrfs/orphan.c        |  36 +++++++--------
>   fs/btrfs/transaction.c   |   2 +-
>   fs/btrfs/tree-log.c      |   4 +-
>   9 files changed, 215 insertions(+), 79 deletions(-)
> ---
> base-commit: 76d8783d4c196f4ac990b97ee9f56d2e6797e173
> change-id: 20260715-btrfs-enomem-988f2cc36ffd
> 
> Best regards,


      parent reply	other threads:[~2026-08-12 23:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 18:14 Jeff Layton
2026-08-11 18:14 ` [PATCH v3 1/6] btrfs: use an on-stack path in btrfs_insert_orphan_item() Jeff Layton
2026-08-20 12:04   ` David Sterba
2026-08-20 12:49     ` Jeff Layton
2026-08-20 22:13     ` Qu Wenruo
2026-08-20 22:59       ` Qu Wenruo
2026-08-21 14:02       ` Jeff Layton
2026-08-11 18:14 ` [PATCH v3 2/6] btrfs: use an on-stack path in btrfs_del_orphan_item() Jeff Layton
2026-08-11 18:14 ` [PATCH v3 3/6] btrfs: split btrfs_insert_delayed_dir_index() into prealloc and commit phases Jeff Layton
2026-08-11 18:14 ` [PATCH v3 4/6] btrfs: pre-allocate delayed dir index before btree modification Jeff Layton
2026-08-11 18:14 ` [PATCH v3 5/6] btrfs: handle ENOMEM from btrfs_insert_dir_item() without aborting Jeff Layton
2026-08-11 18:14 ` [PATCH v3 6/6] btrfs: pre-allocate delayed dir index for non-overwrite rename Jeff Layton
2026-08-12 23:22 ` Qu Wenruo [this message]

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=1943a03e-5569-4a8b-b008-14211517c805@suse.com \
    --to=wqu@suse.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=jlayton@kernel.org \
    --cc=kernel-team@fb.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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

all inboxes | Powered by JetHome®