| Message ID | 20210218165348.4754-4-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs: Delayed Attributes | expand |
On Thu, Feb 18, 2021 at 09:53:29AM -0700, Allison Henderson wrote: > This patch hoists transaction handling in xfs_attr_node_removename to > xfs_attr_node_remove_step. This will help keep transaction handling in > higher level functions instead of buried in subfunctions when we > introduce delay attributes > > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++----------------------- > 1 file changed, 22 insertions(+), 23 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 4e6c89d..3cf76e2 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -1251,9 +1251,7 @@ xfs_attr_node_remove_step( > struct xfs_da_args *args, > struct xfs_da_state *state) > { > - int retval, error; > - struct xfs_inode *dp = args->dp; > - > + int error = 0; > > /* > * If there is an out-of-line value, de-allocate the blocks. > @@ -1265,25 +1263,6 @@ xfs_attr_node_remove_step( > if (error) > return error; > } > - retval = xfs_attr_node_remove_cleanup(args, state); > - > - /* > - * Check to see if the tree needs to be collapsed. > - */ > - if (retval && (state->path.active > 1)) { > - error = xfs_da3_join(state); > - if (error) > - return error; > - error = xfs_defer_finish(&args->trans); > - if (error) > - return error; > - /* > - * Commit the Btree join operation and start a new trans. > - */ > - error = xfs_trans_roll_inode(&args->trans, dp); > - if (error) > - return error; > - } > > return error; > } > @@ -1299,7 +1278,7 @@ xfs_attr_node_removename( > struct xfs_da_args *args) > { > struct xfs_da_state *state = NULL; > - int error; > + int retval, error; > struct xfs_inode *dp = args->dp; > > trace_xfs_attr_node_removename(args); > @@ -1312,6 +1291,26 @@ xfs_attr_node_removename( > if (error) > goto out; > > + retval = xfs_attr_node_remove_cleanup(args, state); > + > + /* > + * Check to see if the tree needs to be collapsed. > + */ > + if (retval && (state->path.active > 1)) { > + error = xfs_da3_join(state); > + if (error) > + goto out; > + error = xfs_defer_finish(&args->trans); > + if (error) > + goto out; > + /* > + * Commit the Btree join operation and start a new trans. > + */ > + error = xfs_trans_roll_inode(&args->trans, dp); > + if (error) > + goto out; > + } > + > /* > * If the result is small enough, push it all into the inode. > */ > -- > 2.7.4 >
On 2/24/21 8:04 AM, Brian Foster wrote: > On Thu, Feb 18, 2021 at 09:53:29AM -0700, Allison Henderson wrote: >> This patch hoists transaction handling in xfs_attr_node_removename to >> xfs_attr_node_remove_step. This will help keep transaction handling in >> higher level functions instead of buried in subfunctions when we >> introduce delay attributes >> >> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> >> --- > > Reviewed-by: Brian Foster <bfoster@redhat.com> Great, thank you! Allison > >> fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++----------------------- >> 1 file changed, 22 insertions(+), 23 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index 4e6c89d..3cf76e2 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c >> @@ -1251,9 +1251,7 @@ xfs_attr_node_remove_step( >> struct xfs_da_args *args, >> struct xfs_da_state *state) >> { >> - int retval, error; >> - struct xfs_inode *dp = args->dp; >> - >> + int error = 0; >> >> /* >> * If there is an out-of-line value, de-allocate the blocks. >> @@ -1265,25 +1263,6 @@ xfs_attr_node_remove_step( >> if (error) >> return error; >> } >> - retval = xfs_attr_node_remove_cleanup(args, state); >> - >> - /* >> - * Check to see if the tree needs to be collapsed. >> - */ >> - if (retval && (state->path.active > 1)) { >> - error = xfs_da3_join(state); >> - if (error) >> - return error; >> - error = xfs_defer_finish(&args->trans); >> - if (error) >> - return error; >> - /* >> - * Commit the Btree join operation and start a new trans. >> - */ >> - error = xfs_trans_roll_inode(&args->trans, dp); >> - if (error) >> - return error; >> - } >> >> return error; >> } >> @@ -1299,7 +1278,7 @@ xfs_attr_node_removename( >> struct xfs_da_args *args) >> { >> struct xfs_da_state *state = NULL; >> - int error; >> + int retval, error; >> struct xfs_inode *dp = args->dp; >> >> trace_xfs_attr_node_removename(args); >> @@ -1312,6 +1291,26 @@ xfs_attr_node_removename( >> if (error) >> goto out; >> >> + retval = xfs_attr_node_remove_cleanup(args, state); >> + >> + /* >> + * Check to see if the tree needs to be collapsed. >> + */ >> + if (retval && (state->path.active > 1)) { >> + error = xfs_da3_join(state); >> + if (error) >> + goto out; >> + error = xfs_defer_finish(&args->trans); >> + if (error) >> + goto out; >> + /* >> + * Commit the Btree join operation and start a new trans. >> + */ >> + error = xfs_trans_roll_inode(&args->trans, dp); >> + if (error) >> + goto out; >> + } >> + >> /* >> * If the result is small enough, push it all into the inode. >> */ >> -- >> 2.7.4 >> >
On Thu, Feb 18, 2021 at 09:53:29AM -0700, Allison Henderson wrote: > This patch hoists transaction handling in xfs_attr_node_removename to > xfs_attr_node_remove_step. This will help keep transaction handling in > higher level functions instead of buried in subfunctions when we > introduce delay attributes > > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> Looks ok, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++----------------------- > 1 file changed, 22 insertions(+), 23 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 4e6c89d..3cf76e2 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -1251,9 +1251,7 @@ xfs_attr_node_remove_step( > struct xfs_da_args *args, > struct xfs_da_state *state) > { > - int retval, error; > - struct xfs_inode *dp = args->dp; > - > + int error = 0; > > /* > * If there is an out-of-line value, de-allocate the blocks. > @@ -1265,25 +1263,6 @@ xfs_attr_node_remove_step( > if (error) > return error; > } > - retval = xfs_attr_node_remove_cleanup(args, state); > - > - /* > - * Check to see if the tree needs to be collapsed. > - */ > - if (retval && (state->path.active > 1)) { > - error = xfs_da3_join(state); > - if (error) > - return error; > - error = xfs_defer_finish(&args->trans); > - if (error) > - return error; > - /* > - * Commit the Btree join operation and start a new trans. > - */ > - error = xfs_trans_roll_inode(&args->trans, dp); > - if (error) > - return error; > - } > > return error; > } > @@ -1299,7 +1278,7 @@ xfs_attr_node_removename( > struct xfs_da_args *args) > { > struct xfs_da_state *state = NULL; > - int error; > + int retval, error; > struct xfs_inode *dp = args->dp; > > trace_xfs_attr_node_removename(args); > @@ -1312,6 +1291,26 @@ xfs_attr_node_removename( > if (error) > goto out; > > + retval = xfs_attr_node_remove_cleanup(args, state); > + > + /* > + * Check to see if the tree needs to be collapsed. > + */ > + if (retval && (state->path.active > 1)) { > + error = xfs_da3_join(state); > + if (error) > + goto out; > + error = xfs_defer_finish(&args->trans); > + if (error) > + goto out; > + /* > + * Commit the Btree join operation and start a new trans. > + */ > + error = xfs_trans_roll_inode(&args->trans, dp); > + if (error) > + goto out; > + } > + > /* > * If the result is small enough, push it all into the inode. > */ > -- > 2.7.4 >
On 2/25/21 8:02 PM, Darrick J. Wong wrote: > On Thu, Feb 18, 2021 at 09:53:29AM -0700, Allison Henderson wrote: >> This patch hoists transaction handling in xfs_attr_node_removename to >> xfs_attr_node_remove_step. This will help keep transaction handling in >> higher level functions instead of buried in subfunctions when we >> introduce delay attributes >> >> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > > Looks ok, > Reviewed-by: Darrick J. Wong <djwong@kernel.org> Great, thank you! Allison > > --D > >> --- >> fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++----------------------- >> 1 file changed, 22 insertions(+), 23 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index 4e6c89d..3cf76e2 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c >> @@ -1251,9 +1251,7 @@ xfs_attr_node_remove_step( >> struct xfs_da_args *args, >> struct xfs_da_state *state) >> { >> - int retval, error; >> - struct xfs_inode *dp = args->dp; >> - >> + int error = 0; >> >> /* >> * If there is an out-of-line value, de-allocate the blocks. >> @@ -1265,25 +1263,6 @@ xfs_attr_node_remove_step( >> if (error) >> return error; >> } >> - retval = xfs_attr_node_remove_cleanup(args, state); >> - >> - /* >> - * Check to see if the tree needs to be collapsed. >> - */ >> - if (retval && (state->path.active > 1)) { >> - error = xfs_da3_join(state); >> - if (error) >> - return error; >> - error = xfs_defer_finish(&args->trans); >> - if (error) >> - return error; >> - /* >> - * Commit the Btree join operation and start a new trans. >> - */ >> - error = xfs_trans_roll_inode(&args->trans, dp); >> - if (error) >> - return error; >> - } >> >> return error; >> } >> @@ -1299,7 +1278,7 @@ xfs_attr_node_removename( >> struct xfs_da_args *args) >> { >> struct xfs_da_state *state = NULL; >> - int error; >> + int retval, error; >> struct xfs_inode *dp = args->dp; >> >> trace_xfs_attr_node_removename(args); >> @@ -1312,6 +1291,26 @@ xfs_attr_node_removename( >> if (error) >> goto out; >> >> + retval = xfs_attr_node_remove_cleanup(args, state); >> + >> + /* >> + * Check to see if the tree needs to be collapsed. >> + */ >> + if (retval && (state->path.active > 1)) { >> + error = xfs_da3_join(state); >> + if (error) >> + goto out; >> + error = xfs_defer_finish(&args->trans); >> + if (error) >> + goto out; >> + /* >> + * Commit the Btree join operation and start a new trans. >> + */ >> + error = xfs_trans_roll_inode(&args->trans, dp); >> + if (error) >> + goto out; >> + } >> + >> /* >> * If the result is small enough, push it all into the inode. >> */ >> -- >> 2.7.4 >>
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 4e6c89d..3cf76e2 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -1251,9 +1251,7 @@ xfs_attr_node_remove_step( struct xfs_da_args *args, struct xfs_da_state *state) { - int retval, error; - struct xfs_inode *dp = args->dp; - + int error = 0; /* * If there is an out-of-line value, de-allocate the blocks. @@ -1265,25 +1263,6 @@ xfs_attr_node_remove_step( if (error) return error; } - retval = xfs_attr_node_remove_cleanup(args, state); - - /* - * Check to see if the tree needs to be collapsed. - */ - if (retval && (state->path.active > 1)) { - error = xfs_da3_join(state); - if (error) - return error; - error = xfs_defer_finish(&args->trans); - if (error) - return error; - /* - * Commit the Btree join operation and start a new trans. - */ - error = xfs_trans_roll_inode(&args->trans, dp); - if (error) - return error; - } return error; } @@ -1299,7 +1278,7 @@ xfs_attr_node_removename( struct xfs_da_args *args) { struct xfs_da_state *state = NULL; - int error; + int retval, error; struct xfs_inode *dp = args->dp; trace_xfs_attr_node_removename(args); @@ -1312,6 +1291,26 @@ xfs_attr_node_removename( if (error) goto out; + retval = xfs_attr_node_remove_cleanup(args, state); + + /* + * Check to see if the tree needs to be collapsed. + */ + if (retval && (state->path.active > 1)) { + error = xfs_da3_join(state); + if (error) + goto out; + error = xfs_defer_finish(&args->trans); + if (error) + goto out; + /* + * Commit the Btree join operation and start a new trans. + */ + error = xfs_trans_roll_inode(&args->trans, dp); + if (error) + goto out; + } + /* * If the result is small enough, push it all into the inode. */
This patch hoists transaction handling in xfs_attr_node_removename to xfs_attr_node_remove_step. This will help keep transaction handling in higher level functions instead of buried in subfunctions when we introduce delay attributes Signed-off-by: Allison Henderson <allison.henderson@oracle.com> --- fs/xfs/libxfs/xfs_attr.c | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-)