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]>