[PATCH v3 0/6] btrfs: handle -ENOMEM errors in some synchronous dirops without aborting
Jeff Layton <[email protected]>
| Newsgroups | org.kernel.vger.linux-btrfs,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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/[email protected]/ Signed-off-by: Jeff Layton <[email protected]> --- 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/[email protected] 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/[email protected] --- 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 <[email protected]>