| Message ID | 20240820170517.528181-2-hch@lst.de (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [1/6] xfs: merge xfs_attr_leaf_try_add into xfs_attr_leaf_addname | expand |
On Tue, Aug 20, 2024 at 07:04:52PM +0200, Christoph Hellwig wrote: > xfs_attr_leaf_try_add is only called by xfs_attr_leaf_addname, and > merging the two will simplify a following error handling fix. > > To facilitate this move the remote block state save/restore helpers up in > the file so that they don't need forward declarations now. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Nice cleanup, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/libxfs/xfs_attr.c | 176 ++++++++++++++++----------------------- > 1 file changed, 74 insertions(+), 102 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index f30bcc64100d56..b9df7a6b1f9d61 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -51,7 +51,6 @@ STATIC int xfs_attr_shortform_addname(xfs_da_args_t *args); > STATIC int xfs_attr_leaf_get(xfs_da_args_t *args); > STATIC int xfs_attr_leaf_removename(xfs_da_args_t *args); > STATIC int xfs_attr_leaf_hasname(struct xfs_da_args *args, struct xfs_buf **bp); > -STATIC int xfs_attr_leaf_try_add(struct xfs_da_args *args); > > /* > * Internal routines when attribute list is more than one block. > @@ -437,6 +436,33 @@ xfs_attr_hashval( > return xfs_attr_hashname(name, namelen); > } > > +/* Save the current remote block info and clear the current pointers. */ > +static void > +xfs_attr_save_rmt_blk( > + struct xfs_da_args *args) > +{ > + args->blkno2 = args->blkno; > + args->index2 = args->index; > + args->rmtblkno2 = args->rmtblkno; > + args->rmtblkcnt2 = args->rmtblkcnt; > + args->rmtvaluelen2 = args->rmtvaluelen; > + args->rmtblkno = 0; > + args->rmtblkcnt = 0; > + args->rmtvaluelen = 0; > +} > + > +/* Set stored info about a remote block */ > +static void > +xfs_attr_restore_rmt_blk( > + struct xfs_da_args *args) > +{ > + args->blkno = args->blkno2; > + args->index = args->index2; > + args->rmtblkno = args->rmtblkno2; > + args->rmtblkcnt = args->rmtblkcnt2; > + args->rmtvaluelen = args->rmtvaluelen2; > +} > + > /* > * PPTR_REPLACE operations require the caller to set the old and new names and > * values explicitly. Update the canonical fields to the new name and value > @@ -482,49 +508,77 @@ xfs_attr_complete_op( > return replace_state; > } > > +/* > + * Try to add an attribute to an inode in leaf form. > + */ > static int > xfs_attr_leaf_addname( > struct xfs_attr_intent *attr) > { > struct xfs_da_args *args = attr->xattri_da_args; > + struct xfs_buf *bp; > int error; > > ASSERT(xfs_attr_is_leaf(args->dp)); > > + error = xfs_attr3_leaf_read(args->trans, args->dp, args->owner, 0, &bp); > + if (error) > + return error; > + > /* > - * Use the leaf buffer we may already hold locked as a result of > - * a sf-to-leaf conversion. > + * Look up the xattr name to set the insertion point for the new xattr. > */ > - error = xfs_attr_leaf_try_add(args); > - > - if (error == -ENOSPC) { > - error = xfs_attr3_leaf_to_node(args); > - if (error) > - return error; > + error = xfs_attr3_leaf_lookup_int(bp, args); > + switch (error) { > + case -ENOATTR: > + if (args->op_flags & XFS_DA_OP_REPLACE) > + goto out_brelse; > + break; > + case -EEXIST: > + if (!(args->op_flags & XFS_DA_OP_REPLACE)) > + goto out_brelse; > > + trace_xfs_attr_leaf_replace(args); > /* > - * We're not in leaf format anymore, so roll the transaction and > - * retry the add to the newly allocated node block. > + * Save the existing remote attr state so that the current > + * values reflect the state of the new attribute we are about to > + * add, not the attribute we just found and will remove later. > */ > - attr->xattri_dela_state = XFS_DAS_NODE_ADD; > - goto out; > + xfs_attr_save_rmt_blk(args); > + break; > + case 0: > + break; > + default: > + goto out_brelse; > } > - if (error) > - return error; > > /* > * We need to commit and roll if we need to allocate remote xattr blocks > * or perform more xattr manipulations. Otherwise there is nothing more > * to do and we can return success. > */ > - if (args->rmtblkno) > + error = xfs_attr3_leaf_add(bp, args); > + if (error) { > + if (error != -ENOSPC) > + return error; > + error = xfs_attr3_leaf_to_node(args); > + if (error) > + return error; > + > + attr->xattri_dela_state = XFS_DAS_NODE_ADD; > + } else if (args->rmtblkno) { > attr->xattri_dela_state = XFS_DAS_LEAF_SET_RMT; > - else > - attr->xattri_dela_state = xfs_attr_complete_op(attr, > - XFS_DAS_LEAF_REPLACE); > -out: > + } else { > + attr->xattri_dela_state = > + xfs_attr_complete_op(attr, XFS_DAS_LEAF_REPLACE); > + } > + > trace_xfs_attr_leaf_addname_return(attr->xattri_dela_state, args->dp); > return error; > + > +out_brelse: > + xfs_trans_brelse(args->trans, bp); > + return error; > } > > /* > @@ -1170,88 +1224,6 @@ xfs_attr_shortform_addname( > * External routines when attribute list is one block > *========================================================================*/ > > -/* Save the current remote block info and clear the current pointers. */ > -static void > -xfs_attr_save_rmt_blk( > - struct xfs_da_args *args) > -{ > - args->blkno2 = args->blkno; > - args->index2 = args->index; > - args->rmtblkno2 = args->rmtblkno; > - args->rmtblkcnt2 = args->rmtblkcnt; > - args->rmtvaluelen2 = args->rmtvaluelen; > - args->rmtblkno = 0; > - args->rmtblkcnt = 0; > - args->rmtvaluelen = 0; > -} > - > -/* Set stored info about a remote block */ > -static void > -xfs_attr_restore_rmt_blk( > - struct xfs_da_args *args) > -{ > - args->blkno = args->blkno2; > - args->index = args->index2; > - args->rmtblkno = args->rmtblkno2; > - args->rmtblkcnt = args->rmtblkcnt2; > - args->rmtvaluelen = args->rmtvaluelen2; > -} > - > -/* > - * Tries to add an attribute to an inode in leaf form > - * > - * This function is meant to execute as part of a delayed operation and leaves > - * the transaction handling to the caller. On success the attribute is added > - * and the inode and transaction are left dirty. If there is not enough space, > - * the attr data is converted to node format and -ENOSPC is returned. Caller is > - * responsible for handling the dirty inode and transaction or adding the attr > - * in node format. > - */ > -STATIC int > -xfs_attr_leaf_try_add( > - struct xfs_da_args *args) > -{ > - struct xfs_buf *bp; > - int error; > - > - error = xfs_attr3_leaf_read(args->trans, args->dp, args->owner, 0, &bp); > - if (error) > - return error; > - > - /* > - * Look up the xattr name to set the insertion point for the new xattr. > - */ > - error = xfs_attr3_leaf_lookup_int(bp, args); > - switch (error) { > - case -ENOATTR: > - if (args->op_flags & XFS_DA_OP_REPLACE) > - goto out_brelse; > - break; > - case -EEXIST: > - if (!(args->op_flags & XFS_DA_OP_REPLACE)) > - goto out_brelse; > - > - trace_xfs_attr_leaf_replace(args); > - /* > - * Save the existing remote attr state so that the current > - * values reflect the state of the new attribute we are about to > - * add, not the attribute we just found and will remove later. > - */ > - xfs_attr_save_rmt_blk(args); > - break; > - case 0: > - break; > - default: > - goto out_brelse; > - } > - > - return xfs_attr3_leaf_add(bp, args); > - > -out_brelse: > - xfs_trans_brelse(args->trans, bp); > - return error; > -} > - > /* > * Return EEXIST if attr is found, or ENOATTR if not > */ > -- > 2.43.0 > >
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index f30bcc64100d56..b9df7a6b1f9d61 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -51,7 +51,6 @@ STATIC int xfs_attr_shortform_addname(xfs_da_args_t *args); STATIC int xfs_attr_leaf_get(xfs_da_args_t *args); STATIC int xfs_attr_leaf_removename(xfs_da_args_t *args); STATIC int xfs_attr_leaf_hasname(struct xfs_da_args *args, struct xfs_buf **bp); -STATIC int xfs_attr_leaf_try_add(struct xfs_da_args *args); /* * Internal routines when attribute list is more than one block. @@ -437,6 +436,33 @@ xfs_attr_hashval( return xfs_attr_hashname(name, namelen); } +/* Save the current remote block info and clear the current pointers. */ +static void +xfs_attr_save_rmt_blk( + struct xfs_da_args *args) +{ + args->blkno2 = args->blkno; + args->index2 = args->index; + args->rmtblkno2 = args->rmtblkno; + args->rmtblkcnt2 = args->rmtblkcnt; + args->rmtvaluelen2 = args->rmtvaluelen; + args->rmtblkno = 0; + args->rmtblkcnt = 0; + args->rmtvaluelen = 0; +} + +/* Set stored info about a remote block */ +static void +xfs_attr_restore_rmt_blk( + struct xfs_da_args *args) +{ + args->blkno = args->blkno2; + args->index = args->index2; + args->rmtblkno = args->rmtblkno2; + args->rmtblkcnt = args->rmtblkcnt2; + args->rmtvaluelen = args->rmtvaluelen2; +} + /* * PPTR_REPLACE operations require the caller to set the old and new names and * values explicitly. Update the canonical fields to the new name and value @@ -482,49 +508,77 @@ xfs_attr_complete_op( return replace_state; } +/* + * Try to add an attribute to an inode in leaf form. + */ static int xfs_attr_leaf_addname( struct xfs_attr_intent *attr) { struct xfs_da_args *args = attr->xattri_da_args; + struct xfs_buf *bp; int error; ASSERT(xfs_attr_is_leaf(args->dp)); + error = xfs_attr3_leaf_read(args->trans, args->dp, args->owner, 0, &bp); + if (error) + return error; + /* - * Use the leaf buffer we may already hold locked as a result of - * a sf-to-leaf conversion. + * Look up the xattr name to set the insertion point for the new xattr. */ - error = xfs_attr_leaf_try_add(args); - - if (error == -ENOSPC) { - error = xfs_attr3_leaf_to_node(args); - if (error) - return error; + error = xfs_attr3_leaf_lookup_int(bp, args); + switch (error) { + case -ENOATTR: + if (args->op_flags & XFS_DA_OP_REPLACE) + goto out_brelse; + break; + case -EEXIST: + if (!(args->op_flags & XFS_DA_OP_REPLACE)) + goto out_brelse; + trace_xfs_attr_leaf_replace(args); /* - * We're not in leaf format anymore, so roll the transaction and - * retry the add to the newly allocated node block. + * Save the existing remote attr state so that the current + * values reflect the state of the new attribute we are about to + * add, not the attribute we just found and will remove later. */ - attr->xattri_dela_state = XFS_DAS_NODE_ADD; - goto out; + xfs_attr_save_rmt_blk(args); + break; + case 0: + break; + default: + goto out_brelse; } - if (error) - return error; /* * We need to commit and roll if we need to allocate remote xattr blocks * or perform more xattr manipulations. Otherwise there is nothing more * to do and we can return success. */ - if (args->rmtblkno) + error = xfs_attr3_leaf_add(bp, args); + if (error) { + if (error != -ENOSPC) + return error; + error = xfs_attr3_leaf_to_node(args); + if (error) + return error; + + attr->xattri_dela_state = XFS_DAS_NODE_ADD; + } else if (args->rmtblkno) { attr->xattri_dela_state = XFS_DAS_LEAF_SET_RMT; - else - attr->xattri_dela_state = xfs_attr_complete_op(attr, - XFS_DAS_LEAF_REPLACE); -out: + } else { + attr->xattri_dela_state = + xfs_attr_complete_op(attr, XFS_DAS_LEAF_REPLACE); + } + trace_xfs_attr_leaf_addname_return(attr->xattri_dela_state, args->dp); return error; + +out_brelse: + xfs_trans_brelse(args->trans, bp); + return error; } /* @@ -1170,88 +1224,6 @@ xfs_attr_shortform_addname( * External routines when attribute list is one block *========================================================================*/ -/* Save the current remote block info and clear the current pointers. */ -static void -xfs_attr_save_rmt_blk( - struct xfs_da_args *args) -{ - args->blkno2 = args->blkno; - args->index2 = args->index; - args->rmtblkno2 = args->rmtblkno; - args->rmtblkcnt2 = args->rmtblkcnt; - args->rmtvaluelen2 = args->rmtvaluelen; - args->rmtblkno = 0; - args->rmtblkcnt = 0; - args->rmtvaluelen = 0; -} - -/* Set stored info about a remote block */ -static void -xfs_attr_restore_rmt_blk( - struct xfs_da_args *args) -{ - args->blkno = args->blkno2; - args->index = args->index2; - args->rmtblkno = args->rmtblkno2; - args->rmtblkcnt = args->rmtblkcnt2; - args->rmtvaluelen = args->rmtvaluelen2; -} - -/* - * Tries to add an attribute to an inode in leaf form - * - * This function is meant to execute as part of a delayed operation and leaves - * the transaction handling to the caller. On success the attribute is added - * and the inode and transaction are left dirty. If there is not enough space, - * the attr data is converted to node format and -ENOSPC is returned. Caller is - * responsible for handling the dirty inode and transaction or adding the attr - * in node format. - */ -STATIC int -xfs_attr_leaf_try_add( - struct xfs_da_args *args) -{ - struct xfs_buf *bp; - int error; - - error = xfs_attr3_leaf_read(args->trans, args->dp, args->owner, 0, &bp); - if (error) - return error; - - /* - * Look up the xattr name to set the insertion point for the new xattr. - */ - error = xfs_attr3_leaf_lookup_int(bp, args); - switch (error) { - case -ENOATTR: - if (args->op_flags & XFS_DA_OP_REPLACE) - goto out_brelse; - break; - case -EEXIST: - if (!(args->op_flags & XFS_DA_OP_REPLACE)) - goto out_brelse; - - trace_xfs_attr_leaf_replace(args); - /* - * Save the existing remote attr state so that the current - * values reflect the state of the new attribute we are about to - * add, not the attribute we just found and will remove later. - */ - xfs_attr_save_rmt_blk(args); - break; - case 0: - break; - default: - goto out_brelse; - } - - return xfs_attr3_leaf_add(bp, args); - -out_brelse: - xfs_trans_brelse(args->trans, bp); - return error; -} - /* * Return EEXIST if attr is found, or ENOATTR if not */
xfs_attr_leaf_try_add is only called by xfs_attr_leaf_addname, and merging the two will simplify a following error handling fix. To facilitate this move the remote block state save/restore helpers up in the file so that they don't need forward declarations now. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/libxfs/xfs_attr.c | 176 ++++++++++++++++----------------------- 1 file changed, 74 insertions(+), 102 deletions(-)