| Message ID | 20210218165348.4754-3-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: Delayed Attributes | expand |
On Thu, Feb 18, 2021 at 09:53:28AM -0700, Allison Henderson wrote: > This patch pulls a new helper function xfs_attr_node_remove_cleanup out > of xfs_attr_node_remove_step. This helps to modularize > xfs_attr_node_remove_step which will help make the delayed attribute > code easier to follow > > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > --- Looks like I sent a review for this on v14... Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/libxfs/xfs_attr.c | 29 ++++++++++++++++++++--------- > 1 file changed, 20 insertions(+), 9 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 28ff93d..4e6c89d 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -1220,6 +1220,25 @@ xfs_attr_node_remove_rmt( > return xfs_attr_refillstate(state); > } > > +STATIC int > +xfs_attr_node_remove_cleanup( > + struct xfs_da_args *args, > + struct xfs_da_state *state) > +{ > + struct xfs_da_state_blk *blk; > + int retval; > + > + /* > + * Remove the name and update the hashvals in the tree. > + */ > + blk = &state->path.blk[state->path.active-1]; > + ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); > + retval = xfs_attr3_leaf_remove(blk->bp, args); > + xfs_da3_fixhashpath(state, &state->path); > + > + return retval; > +} > + > /* > * Remove a name from a B-tree attribute list. > * > @@ -1232,7 +1251,6 @@ xfs_attr_node_remove_step( > struct xfs_da_args *args, > struct xfs_da_state *state) > { > - struct xfs_da_state_blk *blk; > int retval, error; > struct xfs_inode *dp = args->dp; > > @@ -1247,14 +1265,7 @@ xfs_attr_node_remove_step( > if (error) > return error; > } > - > - /* > - * Remove the name and update the hashvals in the tree. > - */ > - blk = &state->path.blk[ state->path.active-1 ]; > - ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); > - retval = xfs_attr3_leaf_remove(blk->bp, args); > - xfs_da3_fixhashpath(state, &state->path); > + retval = xfs_attr_node_remove_cleanup(args, state); > > /* > * Check to see if the tree needs to be collapsed. > -- > 2.7.4 >
On 2/24/21 8:03 AM, Brian Foster wrote: > On Thu, Feb 18, 2021 at 09:53:28AM -0700, Allison Henderson wrote: >> This patch pulls a new helper function xfs_attr_node_remove_cleanup out >> of xfs_attr_node_remove_step. This helps to modularize >> xfs_attr_node_remove_step which will help make the delayed attribute >> code easier to follow >> >> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> >> Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> >> --- > > Looks like I sent a review for this on v14... > > Reviewed-by: Brian Foster <bfoster@redhat.com> Sorry about that, I got tied up in the extra refactoring. Will add. Thanks again! Allison > >> fs/xfs/libxfs/xfs_attr.c | 29 ++++++++++++++++++++--------- >> 1 file changed, 20 insertions(+), 9 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index 28ff93d..4e6c89d 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c >> @@ -1220,6 +1220,25 @@ xfs_attr_node_remove_rmt( >> return xfs_attr_refillstate(state); >> } >> >> +STATIC int >> +xfs_attr_node_remove_cleanup( >> + struct xfs_da_args *args, >> + struct xfs_da_state *state) >> +{ >> + struct xfs_da_state_blk *blk; >> + int retval; >> + >> + /* >> + * Remove the name and update the hashvals in the tree. >> + */ >> + blk = &state->path.blk[state->path.active-1]; >> + ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); >> + retval = xfs_attr3_leaf_remove(blk->bp, args); >> + xfs_da3_fixhashpath(state, &state->path); >> + >> + return retval; >> +} >> + >> /* >> * Remove a name from a B-tree attribute list. >> * >> @@ -1232,7 +1251,6 @@ xfs_attr_node_remove_step( >> struct xfs_da_args *args, >> struct xfs_da_state *state) >> { >> - struct xfs_da_state_blk *blk; >> int retval, error; >> struct xfs_inode *dp = args->dp; >> >> @@ -1247,14 +1265,7 @@ xfs_attr_node_remove_step( >> if (error) >> return error; >> } >> - >> - /* >> - * Remove the name and update the hashvals in the tree. >> - */ >> - blk = &state->path.blk[ state->path.active-1 ]; >> - ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); >> - retval = xfs_attr3_leaf_remove(blk->bp, args); >> - xfs_da3_fixhashpath(state, &state->path); >> + retval = xfs_attr_node_remove_cleanup(args, state); >> >> /* >> * Check to see if the tree needs to be collapsed. >> -- >> 2.7.4 >> >
On Thu, Feb 18, 2021 at 09:53:28AM -0700, Allison Henderson wrote: > This patch pulls a new helper function xfs_attr_node_remove_cleanup out > of xfs_attr_node_remove_step. This helps to modularize > xfs_attr_node_remove_step which will help make the delayed attribute > code easier to follow > > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> Looks ok, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/xfs/libxfs/xfs_attr.c | 29 ++++++++++++++++++++--------- > 1 file changed, 20 insertions(+), 9 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 28ff93d..4e6c89d 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -1220,6 +1220,25 @@ xfs_attr_node_remove_rmt( > return xfs_attr_refillstate(state); > } > > +STATIC int > +xfs_attr_node_remove_cleanup( > + struct xfs_da_args *args, > + struct xfs_da_state *state) > +{ > + struct xfs_da_state_blk *blk; > + int retval; > + > + /* > + * Remove the name and update the hashvals in the tree. > + */ > + blk = &state->path.blk[state->path.active-1]; > + ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); > + retval = xfs_attr3_leaf_remove(blk->bp, args); > + xfs_da3_fixhashpath(state, &state->path); > + > + return retval; > +} > + > /* > * Remove a name from a B-tree attribute list. > * > @@ -1232,7 +1251,6 @@ xfs_attr_node_remove_step( > struct xfs_da_args *args, > struct xfs_da_state *state) > { > - struct xfs_da_state_blk *blk; > int retval, error; > struct xfs_inode *dp = args->dp; > > @@ -1247,14 +1265,7 @@ xfs_attr_node_remove_step( > if (error) > return error; > } > - > - /* > - * Remove the name and update the hashvals in the tree. > - */ > - blk = &state->path.blk[ state->path.active-1 ]; > - ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); > - retval = xfs_attr3_leaf_remove(blk->bp, args); > - xfs_da3_fixhashpath(state, &state->path); > + retval = xfs_attr_node_remove_cleanup(args, state); > > /* > * Check to see if the tree needs to be collapsed. > -- > 2.7.4 >
On 2/25/21 8:00 PM, Darrick J. Wong wrote: > On Thu, Feb 18, 2021 at 09:53:28AM -0700, Allison Henderson wrote: >> This patch pulls a new helper function xfs_attr_node_remove_cleanup out >> of xfs_attr_node_remove_step. This helps to modularize >> xfs_attr_node_remove_step which will help make the delayed attribute >> code easier to follow >> >> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> >> Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com> > > Looks ok, > Reviewed-by: Darrick J. Wong <djwong@kernel.org> Ok, thanks! Allison > > --D > >> --- >> fs/xfs/libxfs/xfs_attr.c | 29 ++++++++++++++++++++--------- >> 1 file changed, 20 insertions(+), 9 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >> index 28ff93d..4e6c89d 100644 >> --- a/fs/xfs/libxfs/xfs_attr.c >> +++ b/fs/xfs/libxfs/xfs_attr.c >> @@ -1220,6 +1220,25 @@ xfs_attr_node_remove_rmt( >> return xfs_attr_refillstate(state); >> } >> >> +STATIC int >> +xfs_attr_node_remove_cleanup( >> + struct xfs_da_args *args, >> + struct xfs_da_state *state) >> +{ >> + struct xfs_da_state_blk *blk; >> + int retval; >> + >> + /* >> + * Remove the name and update the hashvals in the tree. >> + */ >> + blk = &state->path.blk[state->path.active-1]; >> + ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); >> + retval = xfs_attr3_leaf_remove(blk->bp, args); >> + xfs_da3_fixhashpath(state, &state->path); >> + >> + return retval; >> +} >> + >> /* >> * Remove a name from a B-tree attribute list. >> * >> @@ -1232,7 +1251,6 @@ xfs_attr_node_remove_step( >> struct xfs_da_args *args, >> struct xfs_da_state *state) >> { >> - struct xfs_da_state_blk *blk; >> int retval, error; >> struct xfs_inode *dp = args->dp; >> >> @@ -1247,14 +1265,7 @@ xfs_attr_node_remove_step( >> if (error) >> return error; >> } >> - >> - /* >> - * Remove the name and update the hashvals in the tree. >> - */ >> - blk = &state->path.blk[ state->path.active-1 ]; >> - ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); >> - retval = xfs_attr3_leaf_remove(blk->bp, args); >> - xfs_da3_fixhashpath(state, &state->path); >> + retval = xfs_attr_node_remove_cleanup(args, state); >> >> /* >> * Check to see if the tree needs to be collapsed. >> -- >> 2.7.4 >>
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 28ff93d..4e6c89d 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -1220,6 +1220,25 @@ xfs_attr_node_remove_rmt( return xfs_attr_refillstate(state); } +STATIC int +xfs_attr_node_remove_cleanup( + struct xfs_da_args *args, + struct xfs_da_state *state) +{ + struct xfs_da_state_blk *blk; + int retval; + + /* + * Remove the name and update the hashvals in the tree. + */ + blk = &state->path.blk[state->path.active-1]; + ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); + retval = xfs_attr3_leaf_remove(blk->bp, args); + xfs_da3_fixhashpath(state, &state->path); + + return retval; +} + /* * Remove a name from a B-tree attribute list. * @@ -1232,7 +1251,6 @@ xfs_attr_node_remove_step( struct xfs_da_args *args, struct xfs_da_state *state) { - struct xfs_da_state_blk *blk; int retval, error; struct xfs_inode *dp = args->dp; @@ -1247,14 +1265,7 @@ xfs_attr_node_remove_step( if (error) return error; } - - /* - * Remove the name and update the hashvals in the tree. - */ - blk = &state->path.blk[ state->path.active-1 ]; - ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC); - retval = xfs_attr3_leaf_remove(blk->bp, args); - xfs_da3_fixhashpath(state, &state->path); + retval = xfs_attr_node_remove_cleanup(args, state); /* * Check to see if the tree needs to be collapsed.