| Message ID | 20210416092045.2215-9-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: Delay Ready Attributes | expand |
On Fri, Apr 16, 2021 at 02:20:42AM -0700, Allison Henderson wrote: > This patch hoists xfs_attr_leaf_addname into the calling function. The > goal being to get all the code that will require state management into > the same scope. This isn't particularly aesthetic right away, but it is a > preliminary step to merging in the state machine code. > > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > --- > fs/xfs/libxfs/xfs_attr.c | 209 ++++++++++++++++++++++------------------------- > fs/xfs/xfs_trace.h | 1 - > 2 files changed, 96 insertions(+), 114 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 80212d2..5740127 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c ... > @@ -287,10 +288,101 @@ xfs_attr_set_args( > } > > if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) { ... > return error; > > + xfs_attr3_leaf_remove(bp, args); > + > + /* > + * If the result is small enough, shrink it all into the inode. > + */ > + forkoff = xfs_attr_shortform_allfit(bp, dp); > + if (forkoff) > + error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); > + /* bp is gone due to xfs_da_shrink_inode */ > + > + return error; > +node: With the understanding that this mid function return thing goes away before the end of the series: Reviewed-by: Brian Foster <bfoster@redhat.com> > /* > * Promote the attribute list to the Btree format. > */ > @@ -727,115 +819,6 @@ xfs_attr_leaf_try_add( > return retval; > } > > - > -/* > - * Add a name to the leaf attribute list structure > - * > - * This leaf block cannot have a "remote" value, we only call this routine > - * if bmap_one_block() says there is only one block (ie: no remote blks). > - */ > -STATIC int > -xfs_attr_leaf_addname( > - struct xfs_da_args *args) > -{ > - int error, forkoff; > - struct xfs_buf *bp = NULL; > - struct xfs_inode *dp = args->dp; > - > - trace_xfs_attr_leaf_addname(args); > - > - error = xfs_attr_leaf_try_add(args, bp); > - if (error) > - return error; > - > - /* > - * Commit the transaction that added the attr name so that > - * later routines can manage their own transactions. > - */ > - error = xfs_trans_roll_inode(&args->trans, dp); > - if (error) > - return error; > - > - /* > - * If there was an out-of-line value, allocate the blocks we > - * identified for its storage and copy the value. This is done > - * after we create the attribute so that we don't overflow the > - * maximum size of a transaction and/or hit a deadlock. > - */ > - if (args->rmtblkno > 0) { > - error = xfs_attr_rmtval_set(args); > - if (error) > - return error; > - } > - > - if (!(args->op_flags & XFS_DA_OP_RENAME)) { > - /* > - * Added a "remote" value, just clear the incomplete flag. > - */ > - if (args->rmtblkno > 0) > - error = xfs_attr3_leaf_clearflag(args); > - > - return error; > - } > - > - /* > - * If this is an atomic rename operation, we must "flip" the incomplete > - * flags on the "new" and "old" attribute/value pairs so that one > - * disappears and one appears atomically. Then we must remove the "old" > - * attribute/value pair. > - * > - * In a separate transaction, set the incomplete flag on the "old" attr > - * and clear the incomplete flag on the "new" attr. > - */ > - > - error = xfs_attr3_leaf_flipflags(args); > - if (error) > - return error; > - /* > - * Commit the flag value change and start the next trans in series. > - */ > - error = xfs_trans_roll_inode(&args->trans, args->dp); > - if (error) > - return error; > - > - /* > - * Dismantle the "old" attribute/value pair by removing a "remote" value > - * (if it exists). > - */ > - xfs_attr_restore_rmt_blk(args); > - > - if (args->rmtblkno) { > - error = xfs_attr_rmtval_invalidate(args); > - if (error) > - return error; > - > - error = xfs_attr_rmtval_remove(args); > - if (error) > - return error; > - } > - > - /* > - * Read in the block containing the "old" attr, then remove the "old" > - * attr from that block (neat, huh!) > - */ > - error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, > - &bp); > - if (error) > - return error; > - > - xfs_attr3_leaf_remove(bp, args); > - > - /* > - * If the result is small enough, shrink it all into the inode. > - */ > - forkoff = xfs_attr_shortform_allfit(bp, dp); > - if (forkoff) > - error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); > - /* bp is gone due to xfs_da_shrink_inode */ > - > - return error; > -} > - > /* > * Return EEXIST if attr is found, or ENOATTR if not > */ > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h > index 808ae33..3c1c830 100644 > --- a/fs/xfs/xfs_trace.h > +++ b/fs/xfs/xfs_trace.h > @@ -1914,7 +1914,6 @@ DEFINE_ATTR_EVENT(xfs_attr_leaf_add); > DEFINE_ATTR_EVENT(xfs_attr_leaf_add_old); > DEFINE_ATTR_EVENT(xfs_attr_leaf_add_new); > DEFINE_ATTR_EVENT(xfs_attr_leaf_add_work); > -DEFINE_ATTR_EVENT(xfs_attr_leaf_addname); > DEFINE_ATTR_EVENT(xfs_attr_leaf_create); > DEFINE_ATTR_EVENT(xfs_attr_leaf_compact); > DEFINE_ATTR_EVENT(xfs_attr_leaf_get); > -- > 2.7.4 >
On 4/23/21 10:06 AM, Brian Foster wrote: > On Fri, Apr 16, 2021 at 02:20:42AM -0700, Allison Henderson wrote: >> This patch hoists xfs_attr_leaf_addname into the calling function. The >> goal being to get all the code that will require state management into >> the same scope. This isn't particularly aesthetic right away, but it is a >> preliminary step to merging in the state machine code. >> >> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> >> Reviewed-by: Darrick J. Wong <djwong@kernel.org> >> Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> >> --- >> fs/xfs/libxfs/xfs_attr.c | 209 ++++++++++++++++++++++------------------------- >> fs/xfs/xfs_trace.h | 1 - >> 2 files changed, 96 insertions(+), 114 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index 80212d2..5740127 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c > ... >> @@ -287,10 +288,101 @@ xfs_attr_set_args( >> } >> >> if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) { > ... >> return error; >> >> + xfs_attr3_leaf_remove(bp, args); >> + >> + /* >> + * If the result is small enough, shrink it all into the inode. >> + */ >> + forkoff = xfs_attr_shortform_allfit(bp, dp); >> + if (forkoff) >> + error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); >> + /* bp is gone due to xfs_da_shrink_inode */ >> + >> + return error; >> +node: > > With the understanding that this mid function return thing goes away > before the end of the series: > > Reviewed-by: Brian Foster <bfoster@redhat.com> Great, thank you! Allison > >> /* >> * Promote the attribute list to the Btree format. >> */ >> @@ -727,115 +819,6 @@ xfs_attr_leaf_try_add( >> return retval; >> } >> >> - >> -/* >> - * Add a name to the leaf attribute list structure >> - * >> - * This leaf block cannot have a "remote" value, we only call this routine >> - * if bmap_one_block() says there is only one block (ie: no remote blks). >> - */ >> -STATIC int >> -xfs_attr_leaf_addname( >> - struct xfs_da_args *args) >> -{ >> - int error, forkoff; >> - struct xfs_buf *bp = NULL; >> - struct xfs_inode *dp = args->dp; >> - >> - trace_xfs_attr_leaf_addname(args); >> - >> - error = xfs_attr_leaf_try_add(args, bp); >> - if (error) >> - return error; >> - >> - /* >> - * Commit the transaction that added the attr name so that >> - * later routines can manage their own transactions. >> - */ >> - error = xfs_trans_roll_inode(&args->trans, dp); >> - if (error) >> - return error; >> - >> - /* >> - * If there was an out-of-line value, allocate the blocks we >> - * identified for its storage and copy the value. This is done >> - * after we create the attribute so that we don't overflow the >> - * maximum size of a transaction and/or hit a deadlock. >> - */ >> - if (args->rmtblkno > 0) { >> - error = xfs_attr_rmtval_set(args); >> - if (error) >> - return error; >> - } >> - >> - if (!(args->op_flags & XFS_DA_OP_RENAME)) { >> - /* >> - * Added a "remote" value, just clear the incomplete flag. >> - */ >> - if (args->rmtblkno > 0) >> - error = xfs_attr3_leaf_clearflag(args); >> - >> - return error; >> - } >> - >> - /* >> - * If this is an atomic rename operation, we must "flip" the incomplete >> - * flags on the "new" and "old" attribute/value pairs so that one >> - * disappears and one appears atomically. Then we must remove the "old" >> - * attribute/value pair. >> - * >> - * In a separate transaction, set the incomplete flag on the "old" attr >> - * and clear the incomplete flag on the "new" attr. >> - */ >> - >> - error = xfs_attr3_leaf_flipflags(args); >> - if (error) >> - return error; >> - /* >> - * Commit the flag value change and start the next trans in series. >> - */ >> - error = xfs_trans_roll_inode(&args->trans, args->dp); >> - if (error) >> - return error; >> - >> - /* >> - * Dismantle the "old" attribute/value pair by removing a "remote" value >> - * (if it exists). >> - */ >> - xfs_attr_restore_rmt_blk(args); >> - >> - if (args->rmtblkno) { >> - error = xfs_attr_rmtval_invalidate(args); >> - if (error) >> - return error; >> - >> - error = xfs_attr_rmtval_remove(args); >> - if (error) >> - return error; >> - } >> - >> - /* >> - * Read in the block containing the "old" attr, then remove the "old" >> - * attr from that block (neat, huh!) >> - */ >> - error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, >> - &bp); >> - if (error) >> - return error; >> - >> - xfs_attr3_leaf_remove(bp, args); >> - >> - /* >> - * If the result is small enough, shrink it all into the inode. >> - */ >> - forkoff = xfs_attr_shortform_allfit(bp, dp); >> - if (forkoff) >> - error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); >> - /* bp is gone due to xfs_da_shrink_inode */ >> - >> - return error; >> -} >> - >> /* >> * Return EEXIST if attr is found, or ENOATTR if not >> */ >> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h >> index 808ae33..3c1c830 100644 >> --- a/fs/xfs/xfs_trace.h >> +++ b/fs/xfs/xfs_trace.h >> @@ -1914,7 +1914,6 @@ DEFINE_ATTR_EVENT(xfs_attr_leaf_add); >> DEFINE_ATTR_EVENT(xfs_attr_leaf_add_old); >> DEFINE_ATTR_EVENT(xfs_attr_leaf_add_new); >> DEFINE_ATTR_EVENT(xfs_attr_leaf_add_work); >> -DEFINE_ATTR_EVENT(xfs_attr_leaf_addname); >> DEFINE_ATTR_EVENT(xfs_attr_leaf_create); >> DEFINE_ATTR_EVENT(xfs_attr_leaf_compact); >> DEFINE_ATTR_EVENT(xfs_attr_leaf_get); >> -- >> 2.7.4 >> >
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 80212d2..5740127 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -44,9 +44,9 @@ STATIC int xfs_attr_shortform_addname(xfs_da_args_t *args); * Internal routines when attribute list is one block. */ STATIC int xfs_attr_leaf_get(xfs_da_args_t *args); -STATIC int xfs_attr_leaf_addname(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, struct xfs_buf *bp); /* * Internal routines when attribute list is more than one block. @@ -271,8 +271,9 @@ xfs_attr_set_args( struct xfs_da_args *args) { struct xfs_inode *dp = args->dp; + struct xfs_buf *bp = NULL; struct xfs_da_state *state = NULL; - int error = 0; + int forkoff, error = 0; /* * If the attribute list is already in leaf format, jump straight to @@ -287,10 +288,101 @@ xfs_attr_set_args( } if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) { - error = xfs_attr_leaf_addname(args); - if (error != -ENOSPC) + error = xfs_attr_leaf_try_add(args, bp); + if (error == -ENOSPC) + goto node; + else if (error) + return error; + + /* + * Commit the transaction that added the attr name so that + * later routines can manage their own transactions. + */ + error = xfs_trans_roll_inode(&args->trans, dp); + if (error) + return error; + + /* + * If there was an out-of-line value, allocate the blocks we + * identified for its storage and copy the value. This is done + * after we create the attribute so that we don't overflow the + * maximum size of a transaction and/or hit a deadlock. + */ + if (args->rmtblkno > 0) { + error = xfs_attr_rmtval_set(args); + if (error) + return error; + } + + if (!(args->op_flags & XFS_DA_OP_RENAME)) { + /* + * Added a "remote" value, just clear the incomplete + *flag. + */ + if (args->rmtblkno > 0) + error = xfs_attr3_leaf_clearflag(args); + + return error; + } + + /* + * If this is an atomic rename operation, we must "flip" the + * incomplete flags on the "new" and "old" attribute/value pairs + * so that one disappears and one appears atomically. Then we + * must remove the "old" attribute/value pair. + * + * In a separate transaction, set the incomplete flag on the + * "old" attr and clear the incomplete flag on the "new" attr. + */ + + error = xfs_attr3_leaf_flipflags(args); + if (error) + return error; + /* + * Commit the flag value change and start the next trans in + * series. + */ + error = xfs_trans_roll_inode(&args->trans, args->dp); + if (error) + return error; + + /* + * Dismantle the "old" attribute/value pair by removing a + * "remote" value (if it exists). + */ + xfs_attr_restore_rmt_blk(args); + + if (args->rmtblkno) { + error = xfs_attr_rmtval_invalidate(args); + if (error) + return error; + + error = xfs_attr_rmtval_remove(args); + if (error) + return error; + } + + /* + * Read in the block containing the "old" attr, then remove the + * "old" attr from that block (neat, huh!) + */ + error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, + &bp); + if (error) return error; + xfs_attr3_leaf_remove(bp, args); + + /* + * If the result is small enough, shrink it all into the inode. + */ + forkoff = xfs_attr_shortform_allfit(bp, dp); + if (forkoff) + error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); + /* bp is gone due to xfs_da_shrink_inode */ + + return error; +node: /* * Promote the attribute list to the Btree format. */ @@ -727,115 +819,6 @@ xfs_attr_leaf_try_add( return retval; } - -/* - * Add a name to the leaf attribute list structure - * - * This leaf block cannot have a "remote" value, we only call this routine - * if bmap_one_block() says there is only one block (ie: no remote blks). - */ -STATIC int -xfs_attr_leaf_addname( - struct xfs_da_args *args) -{ - int error, forkoff; - struct xfs_buf *bp = NULL; - struct xfs_inode *dp = args->dp; - - trace_xfs_attr_leaf_addname(args); - - error = xfs_attr_leaf_try_add(args, bp); - if (error) - return error; - - /* - * Commit the transaction that added the attr name so that - * later routines can manage their own transactions. - */ - error = xfs_trans_roll_inode(&args->trans, dp); - if (error) - return error; - - /* - * If there was an out-of-line value, allocate the blocks we - * identified for its storage and copy the value. This is done - * after we create the attribute so that we don't overflow the - * maximum size of a transaction and/or hit a deadlock. - */ - if (args->rmtblkno > 0) { - error = xfs_attr_rmtval_set(args); - if (error) - return error; - } - - if (!(args->op_flags & XFS_DA_OP_RENAME)) { - /* - * Added a "remote" value, just clear the incomplete flag. - */ - if (args->rmtblkno > 0) - error = xfs_attr3_leaf_clearflag(args); - - return error; - } - - /* - * If this is an atomic rename operation, we must "flip" the incomplete - * flags on the "new" and "old" attribute/value pairs so that one - * disappears and one appears atomically. Then we must remove the "old" - * attribute/value pair. - * - * In a separate transaction, set the incomplete flag on the "old" attr - * and clear the incomplete flag on the "new" attr. - */ - - error = xfs_attr3_leaf_flipflags(args); - if (error) - return error; - /* - * Commit the flag value change and start the next trans in series. - */ - error = xfs_trans_roll_inode(&args->trans, args->dp); - if (error) - return error; - - /* - * Dismantle the "old" attribute/value pair by removing a "remote" value - * (if it exists). - */ - xfs_attr_restore_rmt_blk(args); - - if (args->rmtblkno) { - error = xfs_attr_rmtval_invalidate(args); - if (error) - return error; - - error = xfs_attr_rmtval_remove(args); - if (error) - return error; - } - - /* - * Read in the block containing the "old" attr, then remove the "old" - * attr from that block (neat, huh!) - */ - error = xfs_attr3_leaf_read(args->trans, args->dp, args->blkno, - &bp); - if (error) - return error; - - xfs_attr3_leaf_remove(bp, args); - - /* - * If the result is small enough, shrink it all into the inode. - */ - forkoff = xfs_attr_shortform_allfit(bp, dp); - if (forkoff) - error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); - /* bp is gone due to xfs_da_shrink_inode */ - - return error; -} - /* * Return EEXIST if attr is found, or ENOATTR if not */ diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index 808ae33..3c1c830 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -1914,7 +1914,6 @@ DEFINE_ATTR_EVENT(xfs_attr_leaf_add); DEFINE_ATTR_EVENT(xfs_attr_leaf_add_old); DEFINE_ATTR_EVENT(xfs_attr_leaf_add_new); DEFINE_ATTR_EVENT(xfs_attr_leaf_add_work); -DEFINE_ATTR_EVENT(xfs_attr_leaf_addname); DEFINE_ATTR_EVENT(xfs_attr_leaf_create); DEFINE_ATTR_EVENT(xfs_attr_leaf_compact); DEFINE_ATTR_EVENT(xfs_attr_leaf_get);