| Message ID | 20190809213726.32336-8-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Delayed Attributes | expand |
On Fri, Aug 09, 2019 at 02:37:15PM -0700, Allison Collins wrote: > Since delayed operations cannot roll transactions, factor > up the transaction handling into the calling function > > Signed-off-by: Allison Collins <allison.henderson@oracle.com> Looks ok, Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > fs/xfs/libxfs/xfs_attr.c | 10 ++++++++++ > fs/xfs/libxfs/xfs_attr_leaf.c | 5 ----- > 2 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 72af8e2..f36c792 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -752,6 +752,11 @@ xfs_attr_leaf_addname( > 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); > > /* > * Dismantle the "old" attribute/value pair by removing > @@ -1090,6 +1095,11 @@ xfs_attr_node_addname( > error = xfs_attr3_leaf_flipflags(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); > > /* > * Dismantle the "old" attribute/value pair by removing > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c > index 8d2e11f..8a6f5df 100644 > --- a/fs/xfs/libxfs/xfs_attr_leaf.c > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c > @@ -2891,10 +2891,5 @@ xfs_attr3_leaf_flipflags( > XFS_DA_LOGRANGE(leaf2, name_rmt, sizeof(*name_rmt))); > } > > - /* > - * Commit the flag value change and start the next trans in series. > - */ > - error = xfs_trans_roll_inode(&args->trans, args->dp); > - > return error; > } > -- > 2.7.4 >
On Mon, Aug 12, 2019 at 09:02:52AM -0700, Darrick J. Wong wrote: > On Fri, Aug 09, 2019 at 02:37:15PM -0700, Allison Collins wrote: > > Since delayed operations cannot roll transactions, factor > > up the transaction handling into the calling function > > > > Signed-off-by: Allison Collins <allison.henderson@oracle.com> > > Looks ok, > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> No, not ok! > > --- > > fs/xfs/libxfs/xfs_attr.c | 10 ++++++++++ > > fs/xfs/libxfs/xfs_attr_leaf.c | 5 ----- > > 2 files changed, 10 insertions(+), 5 deletions(-) > > > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > > index 72af8e2..f36c792 100644 > > --- a/fs/xfs/libxfs/xfs_attr.c > > +++ b/fs/xfs/libxfs/xfs_attr.c > > @@ -752,6 +752,11 @@ xfs_attr_leaf_addname( > > 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); Lost error value here! > > > > /* > > * Dismantle the "old" attribute/value pair by removing > > @@ -1090,6 +1095,11 @@ xfs_attr_node_addname( > > error = xfs_attr3_leaf_flipflags(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); And here! --D > > > > /* > > * Dismantle the "old" attribute/value pair by removing > > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c > > index 8d2e11f..8a6f5df 100644 > > --- a/fs/xfs/libxfs/xfs_attr_leaf.c > > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c > > @@ -2891,10 +2891,5 @@ xfs_attr3_leaf_flipflags( > > XFS_DA_LOGRANGE(leaf2, name_rmt, sizeof(*name_rmt))); > > } > > > > - /* > > - * Commit the flag value change and start the next trans in series. > > - */ > > - error = xfs_trans_roll_inode(&args->trans, args->dp); > > - > > return error; > > } > > -- > > 2.7.4 > >
On 8/12/19 9:30 AM, Darrick J. Wong wrote: > On Mon, Aug 12, 2019 at 09:02:52AM -0700, Darrick J. Wong wrote: >> On Fri, Aug 09, 2019 at 02:37:15PM -0700, Allison Collins wrote: >>> Since delayed operations cannot roll transactions, factor >>> up the transaction handling into the calling function >>> >>> Signed-off-by: Allison Collins <allison.henderson@oracle.com> >> >> Looks ok, >> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > > No, not ok! > >>> --- >>> fs/xfs/libxfs/xfs_attr.c | 10 ++++++++++ >>> fs/xfs/libxfs/xfs_attr_leaf.c | 5 ----- >>> 2 files changed, 10 insertions(+), 5 deletions(-) >>> >>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >>> index 72af8e2..f36c792 100644 >>> --- a/fs/xfs/libxfs/xfs_attr.c >>> +++ b/fs/xfs/libxfs/xfs_attr.c >>> @@ -752,6 +752,11 @@ xfs_attr_leaf_addname( >>> 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); > > Lost error value here! > >>> >>> /* >>> * Dismantle the "old" attribute/value pair by removing >>> @@ -1090,6 +1095,11 @@ xfs_attr_node_addname( >>> error = xfs_attr3_leaf_flipflags(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); > > And here! > > --D Sorry about that! Thanks for the catch! Will add in the error code handlers. Allison > >>> >>> /* >>> * Dismantle the "old" attribute/value pair by removing >>> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c >>> index 8d2e11f..8a6f5df 100644 >>> --- a/fs/xfs/libxfs/xfs_attr_leaf.c >>> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c >>> @@ -2891,10 +2891,5 @@ xfs_attr3_leaf_flipflags( >>> XFS_DA_LOGRANGE(leaf2, name_rmt, sizeof(*name_rmt))); >>> } >>> >>> - /* >>> - * Commit the flag value change and start the next trans in series. >>> - */ >>> - error = xfs_trans_roll_inode(&args->trans, args->dp); >>> - >>> return error; >>> } >>> -- >>> 2.7.4 >>>
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 72af8e2..f36c792 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -752,6 +752,11 @@ xfs_attr_leaf_addname( 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); /* * Dismantle the "old" attribute/value pair by removing @@ -1090,6 +1095,11 @@ xfs_attr_node_addname( error = xfs_attr3_leaf_flipflags(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); /* * Dismantle the "old" attribute/value pair by removing diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c index 8d2e11f..8a6f5df 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -2891,10 +2891,5 @@ xfs_attr3_leaf_flipflags( XFS_DA_LOGRANGE(leaf2, name_rmt, sizeof(*name_rmt))); } - /* - * Commit the flag value change and start the next trans in series. - */ - error = xfs_trans_roll_inode(&args->trans, args->dp); - return error; }
Since delayed operations cannot roll transactions, factor up the transaction handling into the calling function Signed-off-by: Allison Collins <allison.henderson@oracle.com> --- fs/xfs/libxfs/xfs_attr.c | 10 ++++++++++ fs/xfs/libxfs/xfs_attr_leaf.c | 5 ----- 2 files changed, 10 insertions(+), 5 deletions(-)