Message ID | 20200721001606.10781-14-allison.henderson@oracle.com (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
Series | xfs: Delay Ready Attributes | expand |
On Mon, Jul 20, 2020 at 05:15:54PM -0700, Allison Collins wrote: > Some calls to xfs_trans_roll_inode and xfs_defer_finish routines are not > needed. If they are the last operations executed in these functions, and > no further changes are made, then higher level routines will roll or > commit the transactions. > > Signed-off-by: Allison Collins <allison.henderson@oracle.com> > Reviewed-by: Brian Foster <bfoster@redhat.com> Looks decent, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/libxfs/xfs_attr.c | 61 ++++++------------------------------------------ > 1 file changed, 7 insertions(+), 54 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 4eff875..1a78023 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -693,34 +693,15 @@ xfs_attr_leaf_addname( > /* > * If the result is small enough, shrink it all into the inode. > */ > - if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) { > + 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 */ > - if (error) > - return error; > - error = xfs_defer_finish(&args->trans); > - if (error) > - return error; > - } > - > - /* > - * Commit the remove and start the next trans in series. > - */ > - error = xfs_trans_roll_inode(&args->trans, dp); > - > } else if (args->rmtblkno > 0) { > /* > * Added a "remote" value, just clear the incomplete flag. > */ > error = xfs_attr3_leaf_clearflag(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); > } > return error; > } > @@ -780,15 +761,11 @@ xfs_attr_leaf_removename( > /* > * If the result is small enough, shrink it all into the inode. > */ > - if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) { > - error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); > + forkoff = xfs_attr_shortform_allfit(bp, dp); > + if (forkoff) > + return xfs_attr3_leaf_to_shortform(bp, args, forkoff); > /* bp is gone due to xfs_da_shrink_inode */ > - if (error) > - return error; > - error = xfs_defer_finish(&args->trans); > - if (error) > - return error; > - } > + > return 0; > } > > @@ -1070,18 +1047,8 @@ xfs_attr_node_addname( > error = xfs_da3_join(state); > if (error) > goto out; > - error = xfs_defer_finish(&args->trans); > - if (error) > - goto out; > } > > - /* > - * Commit and start the next trans in the chain. > - */ > - error = xfs_trans_roll_inode(&args->trans, dp); > - if (error) > - goto out; > - > } else if (args->rmtblkno > 0) { > /* > * Added a "remote" value, just clear the incomplete flag. > @@ -1089,14 +1056,6 @@ xfs_attr_node_addname( > error = xfs_attr3_leaf_clearflag(args); > if (error) > goto out; > - > - /* > - * Commit the flag value change and start the next trans in > - * series. > - */ > - error = xfs_trans_roll_inode(&args->trans, args->dp); > - if (error) > - goto out; > } > retval = error = 0; > > @@ -1135,16 +1094,10 @@ xfs_attr_node_shrink( > if (forkoff) { > error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); > /* bp is gone due to xfs_da_shrink_inode */ > - if (error) > - return error; > - > - error = xfs_defer_finish(&args->trans); > - if (error) > - return error; > } else > xfs_trans_brelse(args->trans, bp); > > - return 0; > + return error; > } > > /* > -- > 2.7.4 >
On 7/21/20 4:34 PM, Darrick J. Wong wrote: > On Mon, Jul 20, 2020 at 05:15:54PM -0700, Allison Collins wrote: >> Some calls to xfs_trans_roll_inode and xfs_defer_finish routines are not >> needed. If they are the last operations executed in these functions, and >> no further changes are made, then higher level routines will roll or >> commit the transactions. >> >> Signed-off-by: Allison Collins <allison.henderson@oracle.com> >> Reviewed-by: Brian Foster <bfoster@redhat.com> > > Looks decent, > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Alrighty, thanks! Allison > > --D > >> --- >> fs/xfs/libxfs/xfs_attr.c | 61 ++++++------------------------------------------ >> 1 file changed, 7 insertions(+), 54 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index 4eff875..1a78023 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c >> @@ -693,34 +693,15 @@ xfs_attr_leaf_addname( >> /* >> * If the result is small enough, shrink it all into the inode. >> */ >> - if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) { >> + 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 */ >> - if (error) >> - return error; >> - error = xfs_defer_finish(&args->trans); >> - if (error) >> - return error; >> - } >> - >> - /* >> - * Commit the remove and start the next trans in series. >> - */ >> - error = xfs_trans_roll_inode(&args->trans, dp); >> - >> } else if (args->rmtblkno > 0) { >> /* >> * Added a "remote" value, just clear the incomplete flag. >> */ >> error = xfs_attr3_leaf_clearflag(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); >> } >> return error; >> } >> @@ -780,15 +761,11 @@ xfs_attr_leaf_removename( >> /* >> * If the result is small enough, shrink it all into the inode. >> */ >> - if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) { >> - error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); >> + forkoff = xfs_attr_shortform_allfit(bp, dp); >> + if (forkoff) >> + return xfs_attr3_leaf_to_shortform(bp, args, forkoff); >> /* bp is gone due to xfs_da_shrink_inode */ >> - if (error) >> - return error; >> - error = xfs_defer_finish(&args->trans); >> - if (error) >> - return error; >> - } >> + >> return 0; >> } >> >> @@ -1070,18 +1047,8 @@ xfs_attr_node_addname( >> error = xfs_da3_join(state); >> if (error) >> goto out; >> - error = xfs_defer_finish(&args->trans); >> - if (error) >> - goto out; >> } >> >> - /* >> - * Commit and start the next trans in the chain. >> - */ >> - error = xfs_trans_roll_inode(&args->trans, dp); >> - if (error) >> - goto out; >> - >> } else if (args->rmtblkno > 0) { >> /* >> * Added a "remote" value, just clear the incomplete flag. >> @@ -1089,14 +1056,6 @@ xfs_attr_node_addname( >> error = xfs_attr3_leaf_clearflag(args); >> if (error) >> goto out; >> - >> - /* >> - * Commit the flag value change and start the next trans in >> - * series. >> - */ >> - error = xfs_trans_roll_inode(&args->trans, args->dp); >> - if (error) >> - goto out; >> } >> retval = error = 0; >> >> @@ -1135,16 +1094,10 @@ xfs_attr_node_shrink( >> if (forkoff) { >> error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); >> /* bp is gone due to xfs_da_shrink_inode */ >> - if (error) >> - return error; >> - >> - error = xfs_defer_finish(&args->trans); >> - if (error) >> - return error; >> } else >> xfs_trans_brelse(args->trans, bp); >> >> - return 0; >> + return error; >> } >> >> /* >> -- >> 2.7.4 >>
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 4eff875..1a78023 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -693,34 +693,15 @@ xfs_attr_leaf_addname( /* * If the result is small enough, shrink it all into the inode. */ - if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) { + 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 */ - if (error) - return error; - error = xfs_defer_finish(&args->trans); - if (error) - return error; - } - - /* - * Commit the remove and start the next trans in series. - */ - error = xfs_trans_roll_inode(&args->trans, dp); - } else if (args->rmtblkno > 0) { /* * Added a "remote" value, just clear the incomplete flag. */ error = xfs_attr3_leaf_clearflag(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); } return error; } @@ -780,15 +761,11 @@ xfs_attr_leaf_removename( /* * If the result is small enough, shrink it all into the inode. */ - if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) { - error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); + forkoff = xfs_attr_shortform_allfit(bp, dp); + if (forkoff) + return xfs_attr3_leaf_to_shortform(bp, args, forkoff); /* bp is gone due to xfs_da_shrink_inode */ - if (error) - return error; - error = xfs_defer_finish(&args->trans); - if (error) - return error; - } + return 0; } @@ -1070,18 +1047,8 @@ xfs_attr_node_addname( error = xfs_da3_join(state); if (error) goto out; - error = xfs_defer_finish(&args->trans); - if (error) - goto out; } - /* - * Commit and start the next trans in the chain. - */ - error = xfs_trans_roll_inode(&args->trans, dp); - if (error) - goto out; - } else if (args->rmtblkno > 0) { /* * Added a "remote" value, just clear the incomplete flag. @@ -1089,14 +1056,6 @@ xfs_attr_node_addname( error = xfs_attr3_leaf_clearflag(args); if (error) goto out; - - /* - * Commit the flag value change and start the next trans in - * series. - */ - error = xfs_trans_roll_inode(&args->trans, args->dp); - if (error) - goto out; } retval = error = 0; @@ -1135,16 +1094,10 @@ xfs_attr_node_shrink( if (forkoff) { error = xfs_attr3_leaf_to_shortform(bp, args, forkoff); /* bp is gone due to xfs_da_shrink_inode */ - if (error) - return error; - - error = xfs_defer_finish(&args->trans); - if (error) - return error; } else xfs_trans_brelse(args->trans, bp); - return 0; + return error; } /*