| Message ID | 20201218072917.16805-2-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: Delayed Attributes | expand |
On Fri, 18 Dec 2020 00:29:03 -0700, Allison Henderson wrote: > From: Allison Collins <allison.henderson@oracle.com> > > This patch as a new helper function xfs_attr_node_remove_step. This The above should probably be "This patch adds a new ...". > will help simplify and modularize the calling function > xfs_attr_node_remove. The calling function is xfs_attr_node_removename. Other than the above mentioned nits, the changes look good to me, Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/libxfs/xfs_attr.c | 46 ++++++++++++++++++++++++++++++++++------------ > 1 file changed, 34 insertions(+), 12 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index fd8e641..8b55a8d 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -1228,19 +1228,14 @@ xfs_attr_node_remove_rmt( > * the root node (a special case of an intermediate node). > */ > STATIC int > -xfs_attr_node_removename( > - struct xfs_da_args *args) > +xfs_attr_node_remove_step( > + struct xfs_da_args *args, > + struct xfs_da_state *state) > { > - struct xfs_da_state *state; > struct xfs_da_state_blk *blk; > int retval, error; > struct xfs_inode *dp = args->dp; > > - trace_xfs_attr_node_removename(args); > - > - error = xfs_attr_node_removename_setup(args, &state); > - if (error) > - goto out; > > /* > * If there is an out-of-line value, de-allocate the blocks. > @@ -1250,7 +1245,7 @@ xfs_attr_node_removename( > if (args->rmtblkno > 0) { > error = xfs_attr_node_remove_rmt(args, state); > if (error) > - goto out; > + return error; > } > > /* > @@ -1267,18 +1262,45 @@ xfs_attr_node_removename( > if (retval && (state->path.active > 1)) { > error = xfs_da3_join(state); > if (error) > - goto out; > + return error; > error = xfs_defer_finish(&args->trans); > if (error) > - goto out; > + return error; > /* > * Commit the Btree join operation and start a new trans. > */ > error = xfs_trans_roll_inode(&args->trans, dp); > if (error) > - goto out; > + return error; > } > > + return error; > +} > + > +/* > + * Remove a name from a B-tree attribute list. > + * > + * This routine will find the blocks of the name to remove, remove them and > + * shrink the tree if needed. > + */ > +STATIC int > +xfs_attr_node_removename( > + struct xfs_da_args *args) > +{ > + struct xfs_da_state *state = NULL; > + int error; > + struct xfs_inode *dp = args->dp; > + > + trace_xfs_attr_node_removename(args); > + > + error = xfs_attr_node_removename_setup(args, &state); > + if (error) > + goto out; > + > + error = xfs_attr_node_remove_step(args, state); > + if (error) > + goto out; > + > /* > * If the result is small enough, push it all into the inode. > */ >
On 12/20/20 11:45 PM, Chandan Babu R wrote: > On Fri, 18 Dec 2020 00:29:03 -0700, Allison Henderson wrote: >> From: Allison Collins <allison.henderson@oracle.com> >> >> This patch as a new helper function xfs_attr_node_remove_step. This > > The above should probably be "This patch adds a new ...". > >> will help simplify and modularize the calling function >> xfs_attr_node_remove. > > The calling function is xfs_attr_node_removename. > > Other than the above mentioned nits, the changes look good to me, > > Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> Ok, will fix nits Thank you! Allison > >> >> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> >> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> >> --- >> fs/xfs/libxfs/xfs_attr.c | 46 ++++++++++++++++++++++++++++++++++------------ >> 1 file changed, 34 insertions(+), 12 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index fd8e641..8b55a8d 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c >> @@ -1228,19 +1228,14 @@ xfs_attr_node_remove_rmt( >> * the root node (a special case of an intermediate node). >> */ >> STATIC int >> -xfs_attr_node_removename( >> - struct xfs_da_args *args) >> +xfs_attr_node_remove_step( >> + struct xfs_da_args *args, >> + struct xfs_da_state *state) >> { >> - struct xfs_da_state *state; >> struct xfs_da_state_blk *blk; >> int retval, error; >> struct xfs_inode *dp = args->dp; >> >> - trace_xfs_attr_node_removename(args); >> - >> - error = xfs_attr_node_removename_setup(args, &state); >> - if (error) >> - goto out; >> >> /* >> * If there is an out-of-line value, de-allocate the blocks. >> @@ -1250,7 +1245,7 @@ xfs_attr_node_removename( >> if (args->rmtblkno > 0) { >> error = xfs_attr_node_remove_rmt(args, state); >> if (error) >> - goto out; >> + return error; >> } >> >> /* >> @@ -1267,18 +1262,45 @@ xfs_attr_node_removename( >> if (retval && (state->path.active > 1)) { >> error = xfs_da3_join(state); >> if (error) >> - goto out; >> + return error; >> error = xfs_defer_finish(&args->trans); >> if (error) >> - goto out; >> + return error; >> /* >> * Commit the Btree join operation and start a new trans. >> */ >> error = xfs_trans_roll_inode(&args->trans, dp); >> if (error) >> - goto out; >> + return error; >> } >> >> + return error; >> +} >> + >> +/* >> + * Remove a name from a B-tree attribute list. >> + * >> + * This routine will find the blocks of the name to remove, remove them and >> + * shrink the tree if needed. >> + */ >> +STATIC int >> +xfs_attr_node_removename( >> + struct xfs_da_args *args) >> +{ >> + struct xfs_da_state *state = NULL; >> + int error; >> + struct xfs_inode *dp = args->dp; >> + >> + trace_xfs_attr_node_removename(args); >> + >> + error = xfs_attr_node_removename_setup(args, &state); >> + if (error) >> + goto out; >> + >> + error = xfs_attr_node_remove_step(args, state); >> + if (error) >> + goto out; >> + >> /* >> * If the result is small enough, push it all into the inode. >> */ >> > >
On Fri, Dec 18, 2020 at 12:29:03AM -0700, Allison Henderson wrote: > From: Allison Collins <allison.henderson@oracle.com> > > This patch as a new helper function xfs_attr_node_remove_step. This > will help simplify and modularize the calling function > xfs_attr_node_remove. > > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> > --- Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/libxfs/xfs_attr.c | 46 ++++++++++++++++++++++++++++++++++------------ > 1 file changed, 34 insertions(+), 12 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index fd8e641..8b55a8d 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -1228,19 +1228,14 @@ xfs_attr_node_remove_rmt( > * the root node (a special case of an intermediate node). > */ > STATIC int > -xfs_attr_node_removename( > - struct xfs_da_args *args) > +xfs_attr_node_remove_step( > + struct xfs_da_args *args, > + struct xfs_da_state *state) > { > - struct xfs_da_state *state; > struct xfs_da_state_blk *blk; > int retval, error; > struct xfs_inode *dp = args->dp; > > - trace_xfs_attr_node_removename(args); > - > - error = xfs_attr_node_removename_setup(args, &state); > - if (error) > - goto out; > > /* > * If there is an out-of-line value, de-allocate the blocks. > @@ -1250,7 +1245,7 @@ xfs_attr_node_removename( > if (args->rmtblkno > 0) { > error = xfs_attr_node_remove_rmt(args, state); > if (error) > - goto out; > + return error; > } > > /* > @@ -1267,18 +1262,45 @@ xfs_attr_node_removename( > if (retval && (state->path.active > 1)) { > error = xfs_da3_join(state); > if (error) > - goto out; > + return error; > error = xfs_defer_finish(&args->trans); > if (error) > - goto out; > + return error; > /* > * Commit the Btree join operation and start a new trans. > */ > error = xfs_trans_roll_inode(&args->trans, dp); > if (error) > - goto out; > + return error; > } > > + return error; > +} > + > +/* > + * Remove a name from a B-tree attribute list. > + * > + * This routine will find the blocks of the name to remove, remove them and > + * shrink the tree if needed. > + */ > +STATIC int > +xfs_attr_node_removename( > + struct xfs_da_args *args) > +{ > + struct xfs_da_state *state = NULL; > + int error; > + struct xfs_inode *dp = args->dp; > + > + trace_xfs_attr_node_removename(args); > + > + error = xfs_attr_node_removename_setup(args, &state); > + if (error) > + goto out; > + > + error = xfs_attr_node_remove_step(args, state); > + 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 fd8e641..8b55a8d 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -1228,19 +1228,14 @@ xfs_attr_node_remove_rmt( * the root node (a special case of an intermediate node). */ STATIC int -xfs_attr_node_removename( - struct xfs_da_args *args) +xfs_attr_node_remove_step( + struct xfs_da_args *args, + struct xfs_da_state *state) { - struct xfs_da_state *state; struct xfs_da_state_blk *blk; int retval, error; struct xfs_inode *dp = args->dp; - trace_xfs_attr_node_removename(args); - - error = xfs_attr_node_removename_setup(args, &state); - if (error) - goto out; /* * If there is an out-of-line value, de-allocate the blocks. @@ -1250,7 +1245,7 @@ xfs_attr_node_removename( if (args->rmtblkno > 0) { error = xfs_attr_node_remove_rmt(args, state); if (error) - goto out; + return error; } /* @@ -1267,18 +1262,45 @@ xfs_attr_node_removename( if (retval && (state->path.active > 1)) { error = xfs_da3_join(state); if (error) - goto out; + return error; error = xfs_defer_finish(&args->trans); if (error) - goto out; + return error; /* * Commit the Btree join operation and start a new trans. */ error = xfs_trans_roll_inode(&args->trans, dp); if (error) - goto out; + return error; } + return error; +} + +/* + * Remove a name from a B-tree attribute list. + * + * This routine will find the blocks of the name to remove, remove them and + * shrink the tree if needed. + */ +STATIC int +xfs_attr_node_removename( + struct xfs_da_args *args) +{ + struct xfs_da_state *state = NULL; + int error; + struct xfs_inode *dp = args->dp; + + trace_xfs_attr_node_removename(args); + + error = xfs_attr_node_removename_setup(args, &state); + if (error) + goto out; + + error = xfs_attr_node_remove_step(args, state); + if (error) + goto out; + /* * If the result is small enough, push it all into the inode. */