[PATCH v3 0/6] btrfs: handle -ENOMEM errors in some synchronous dirops without aborting

Jeff Layton posted 6 patches 1 month, 2 weeks ago
There is a newer version of this series
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(-)
[PATCH v3 0/6] btrfs: handle -ENOMEM errors in some synchronous dirops without aborting
Posted by Jeff Layton 1 month, 2 weeks ago
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>
---
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,
-- 
Jeff Layton <jlayton@kernel.org>
Re: [PATCH v3 0/6] btrfs: handle -ENOMEM errors in some synchronous dirops without aborting
Posted by Qu Wenruo 1 month, 2 weeks ago

在 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,