| Message ID | 20191212041513.13855-7-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | xfs: Delay Ready Attributes | expand |
What does "factor up" mean? Basically this moves the xfs_trans_roll_inode from xfs_attr3_leaf_flipflags to the callers, so I'd expect the subject to mention that. > + /* > + * Commit the flag value change and start the next trans in > + * series. > + */ > + error = xfs_trans_roll_inode(&args->trans, args->dp); Do we really still need these comments? > diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c > index ef96971..4fffa84 100644 > --- a/fs/xfs/libxfs/xfs_attr_leaf.c > +++ b/fs/xfs/libxfs/xfs_attr_leaf.c > @@ -2972,10 +2972,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; This can become a return 0; now.
On 12/24/19 5:16 AM, Christoph Hellwig wrote: > What does "factor up" mean? Basically this moves the > xfs_trans_roll_inode from xfs_attr3_leaf_flipflags to the callers, > so I'd expect the subject to mention that. Yes, that is what I meant for it to mean. I guess I've been on a few other projects that used that terminology, so I didn't think much of it, and this is the first time someone has indicated confusion over it. I can certainly change the verbiage if people prefer. How about "Move up"? "Pull up"? > >> + /* >> + * Commit the flag value change and start the next trans in >> + * series. >> + */ >> + error = xfs_trans_roll_inode(&args->trans, args->dp); > > Do we really still need these comments? I guess I brought it with the corresponding code because I wasn't sure if people would miss it or not. It does seem to be explaining why we're doing a roll right now. We may need to wait for after the holiday break for more people to chime in. > >> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c >> index ef96971..4fffa84 100644 >> --- a/fs/xfs/libxfs/xfs_attr_leaf.c >> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c >> @@ -2972,10 +2972,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; > > This can become a > > return 0; > > now. > Sure, will change. Thanks! Allison
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 88de43c..ee973d2 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -726,6 +726,13 @@ 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); + if (error) + return error; /* * Dismantle the "old" attribute/value pair by removing @@ -1064,6 +1071,13 @@ 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); + if (error) + goto out; /* * 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 ef96971..4fffa84 100644 --- a/fs/xfs/libxfs/xfs_attr_leaf.c +++ b/fs/xfs/libxfs/xfs_attr_leaf.c @@ -2972,10 +2972,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; }