| Message ID | 20200722090518.214624-6-cmaiolino@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | Continue xfs kmem cleanup - V3 | expand |
On Wed, Jul 22, 2020 at 11:05:18AM +0200, Carlos Maiolino wrote: > Every call to xfs_da_state_alloc() also requires setting up state->args > and state->mp > > Change xfs_da_state_alloc() to receive an xfs_da_args_t as argument and > return a xfs_da_state_t with both args and mp already set. I would have not use the typedef versions and switched to the raw struct type, but that's something we should eventually take care of with a bulk cleanup anyway, so: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Jul 22, 2020 at 11:05:18AM +0200, Carlos Maiolino wrote: > Every call to xfs_da_state_alloc() also requires setting up state->args > and state->mp > > Change xfs_da_state_alloc() to receive an xfs_da_args_t as argument and > return a xfs_da_state_t with both args and mp already set. > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> > --- > > Changelog: > V3: > - Originally this patch removed xfs_da_state_alloc(), > per hch's suggestion, instead of removing, it has been > refactored, to also set state->{args,mp} which removes > a few lines of code. > > fs/xfs/libxfs/xfs_attr.c | 17 +++++------------ > fs/xfs/libxfs/xfs_da_btree.c | 8 ++++++-- > fs/xfs/libxfs/xfs_da_btree.h | 2 +- > fs/xfs/libxfs/xfs_dir2_node.c | 17 +++++------------ > fs/xfs/scrub/dabtree.c | 4 +--- > 5 files changed, 18 insertions(+), 30 deletions(-) With the typedefs fixed, Reviewed-by: Dave Chinner <dchinner@redhat.com>
On Wed, Jul 22, 2020 at 11:05:18AM +0200, Carlos Maiolino wrote: > Every call to xfs_da_state_alloc() also requires setting up state->args > and state->mp > > Change xfs_da_state_alloc() to receive an xfs_da_args_t as argument and > return a xfs_da_state_t with both args and mp already set. > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Will fix the typedef thing for xfs_da_state_alloc when I commit this; Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --- > > Changelog: > V3: > - Originally this patch removed xfs_da_state_alloc(), > per hch's suggestion, instead of removing, it has been > refactored, to also set state->{args,mp} which removes > a few lines of code. > > fs/xfs/libxfs/xfs_attr.c | 17 +++++------------ > fs/xfs/libxfs/xfs_da_btree.c | 8 ++++++-- > fs/xfs/libxfs/xfs_da_btree.h | 2 +- > fs/xfs/libxfs/xfs_dir2_node.c | 17 +++++------------ > fs/xfs/scrub/dabtree.c | 4 +--- > 5 files changed, 18 insertions(+), 30 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c > index 3b1bd6e112f89..52e01fc0c5d04 100644 > --- a/fs/xfs/libxfs/xfs_attr.c > +++ b/fs/xfs/libxfs/xfs_attr.c > @@ -750,9 +750,7 @@ xfs_attr_node_addname( > dp = args->dp; > mp = dp->i_mount; > restart: > - state = xfs_da_state_alloc(); > - state->args = args; > - state->mp = mp; > + state = xfs_da_state_alloc(args); > > /* > * Search to see if name already exists, and get back a pointer > @@ -899,9 +897,8 @@ xfs_attr_node_addname( > * attr, not the "new" one. > */ > args->attr_filter |= XFS_ATTR_INCOMPLETE; > - state = xfs_da_state_alloc(); > - state->args = args; > - state->mp = mp; > + state = xfs_da_state_alloc(args); > + > state->inleaf = 0; > error = xfs_da3_node_lookup_int(state, &retval); > if (error) > @@ -975,9 +972,7 @@ xfs_attr_node_removename( > * Tie a string around our finger to remind us where we are. > */ > dp = args->dp; > - state = xfs_da_state_alloc(); > - state->args = args; > - state->mp = dp->i_mount; > + state = xfs_da_state_alloc(args); > > /* > * Search to see if name exists, and get back a pointer to it. > @@ -1207,9 +1202,7 @@ xfs_attr_node_get(xfs_da_args_t *args) > > trace_xfs_attr_node_get(args); > > - state = xfs_da_state_alloc(); > - state->args = args; > - state->mp = args->dp->i_mount; > + state = xfs_da_state_alloc(args); > > /* > * Search to see if name exists, and get back a pointer to it. > diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c > index a4e1f01daf3d8..47da88154b434 100644 > --- a/fs/xfs/libxfs/xfs_da_btree.c > +++ b/fs/xfs/libxfs/xfs_da_btree.c > @@ -79,9 +79,13 @@ kmem_zone_t *xfs_da_state_zone; /* anchor for state struct zone */ > * We don't put them on the stack since they're large. > */ > xfs_da_state_t * > -xfs_da_state_alloc(void) > +xfs_da_state_alloc(xfs_da_args_t *args) > { > - return kmem_cache_zalloc(xfs_da_state_zone, GFP_NOFS | __GFP_NOFAIL); > + xfs_da_state_t *state = kmem_cache_zalloc(xfs_da_state_zone, > + GFP_NOFS | __GFP_NOFAIL); > + state->args = args; > + state->mp = args->dp->i_mount; > + return state; > } > > /* > diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h > index 6e25de6621e4f..bb039dcb0cce4 100644 > --- a/fs/xfs/libxfs/xfs_da_btree.h > +++ b/fs/xfs/libxfs/xfs_da_btree.h > @@ -219,7 +219,7 @@ enum xfs_dacmp xfs_da_compname(struct xfs_da_args *args, > const unsigned char *name, int len); > > > -xfs_da_state_t *xfs_da_state_alloc(void); > +xfs_da_state_t *xfs_da_state_alloc(xfs_da_args_t *args); > void xfs_da_state_free(xfs_da_state_t *state); > > void xfs_da3_node_hdr_from_disk(struct xfs_mount *mp, > diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c > index 6ac4aad98cd76..5d51265d29d6f 100644 > --- a/fs/xfs/libxfs/xfs_dir2_node.c > +++ b/fs/xfs/libxfs/xfs_dir2_node.c > @@ -2015,9 +2015,7 @@ xfs_dir2_node_addname( > /* > * Allocate and initialize the state (btree cursor). > */ > - state = xfs_da_state_alloc(); > - state->args = args; > - state->mp = args->dp->i_mount; > + state = xfs_da_state_alloc(args); > /* > * Look up the name. We're not supposed to find it, but > * this gives us the insertion point. > @@ -2086,9 +2084,8 @@ xfs_dir2_node_lookup( > /* > * Allocate and initialize the btree cursor. > */ > - state = xfs_da_state_alloc(); > - state->args = args; > - state->mp = args->dp->i_mount; > + state = xfs_da_state_alloc(args); > + > /* > * Fill in the path to the entry in the cursor. > */ > @@ -2139,9 +2136,7 @@ xfs_dir2_node_removename( > /* > * Allocate and initialize the btree cursor. > */ > - state = xfs_da_state_alloc(); > - state->args = args; > - state->mp = args->dp->i_mount; > + state = xfs_da_state_alloc(args); > > /* Look up the entry we're deleting, set up the cursor. */ > error = xfs_da3_node_lookup_int(state, &rval); > @@ -2206,9 +2201,7 @@ xfs_dir2_node_replace( > /* > * Allocate and initialize the btree cursor. > */ > - state = xfs_da_state_alloc(); > - state->args = args; > - state->mp = args->dp->i_mount; > + state = xfs_da_state_alloc(args); > > /* > * We have to save new inode number and ftype since > diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c > index 44b15015021f3..e56786f0a13c8 100644 > --- a/fs/xfs/scrub/dabtree.c > +++ b/fs/xfs/scrub/dabtree.c > @@ -476,9 +476,7 @@ xchk_da_btree( > ds.dargs.whichfork = whichfork; > ds.dargs.trans = sc->tp; > ds.dargs.op_flags = XFS_DA_OP_OKNOENT; > - ds.state = xfs_da_state_alloc(); > - ds.state->args = &ds.dargs; > - ds.state->mp = mp; > + ds.state = xfs_da_state_alloc(&ds.dargs); > ds.sc = sc; > ds.private = private; > if (whichfork == XFS_ATTR_FORK) { > -- > 2.26.2 >
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c index 3b1bd6e112f89..52e01fc0c5d04 100644 --- a/fs/xfs/libxfs/xfs_attr.c +++ b/fs/xfs/libxfs/xfs_attr.c @@ -750,9 +750,7 @@ xfs_attr_node_addname( dp = args->dp; mp = dp->i_mount; restart: - state = xfs_da_state_alloc(); - state->args = args; - state->mp = mp; + state = xfs_da_state_alloc(args); /* * Search to see if name already exists, and get back a pointer @@ -899,9 +897,8 @@ xfs_attr_node_addname( * attr, not the "new" one. */ args->attr_filter |= XFS_ATTR_INCOMPLETE; - state = xfs_da_state_alloc(); - state->args = args; - state->mp = mp; + state = xfs_da_state_alloc(args); + state->inleaf = 0; error = xfs_da3_node_lookup_int(state, &retval); if (error) @@ -975,9 +972,7 @@ xfs_attr_node_removename( * Tie a string around our finger to remind us where we are. */ dp = args->dp; - state = xfs_da_state_alloc(); - state->args = args; - state->mp = dp->i_mount; + state = xfs_da_state_alloc(args); /* * Search to see if name exists, and get back a pointer to it. @@ -1207,9 +1202,7 @@ xfs_attr_node_get(xfs_da_args_t *args) trace_xfs_attr_node_get(args); - state = xfs_da_state_alloc(); - state->args = args; - state->mp = args->dp->i_mount; + state = xfs_da_state_alloc(args); /* * Search to see if name exists, and get back a pointer to it. diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c index a4e1f01daf3d8..47da88154b434 100644 --- a/fs/xfs/libxfs/xfs_da_btree.c +++ b/fs/xfs/libxfs/xfs_da_btree.c @@ -79,9 +79,13 @@ kmem_zone_t *xfs_da_state_zone; /* anchor for state struct zone */ * We don't put them on the stack since they're large. */ xfs_da_state_t * -xfs_da_state_alloc(void) +xfs_da_state_alloc(xfs_da_args_t *args) { - return kmem_cache_zalloc(xfs_da_state_zone, GFP_NOFS | __GFP_NOFAIL); + xfs_da_state_t *state = kmem_cache_zalloc(xfs_da_state_zone, + GFP_NOFS | __GFP_NOFAIL); + state->args = args; + state->mp = args->dp->i_mount; + return state; } /* diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h index 6e25de6621e4f..bb039dcb0cce4 100644 --- a/fs/xfs/libxfs/xfs_da_btree.h +++ b/fs/xfs/libxfs/xfs_da_btree.h @@ -219,7 +219,7 @@ enum xfs_dacmp xfs_da_compname(struct xfs_da_args *args, const unsigned char *name, int len); -xfs_da_state_t *xfs_da_state_alloc(void); +xfs_da_state_t *xfs_da_state_alloc(xfs_da_args_t *args); void xfs_da_state_free(xfs_da_state_t *state); void xfs_da3_node_hdr_from_disk(struct xfs_mount *mp, diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c index 6ac4aad98cd76..5d51265d29d6f 100644 --- a/fs/xfs/libxfs/xfs_dir2_node.c +++ b/fs/xfs/libxfs/xfs_dir2_node.c @@ -2015,9 +2015,7 @@ xfs_dir2_node_addname( /* * Allocate and initialize the state (btree cursor). */ - state = xfs_da_state_alloc(); - state->args = args; - state->mp = args->dp->i_mount; + state = xfs_da_state_alloc(args); /* * Look up the name. We're not supposed to find it, but * this gives us the insertion point. @@ -2086,9 +2084,8 @@ xfs_dir2_node_lookup( /* * Allocate and initialize the btree cursor. */ - state = xfs_da_state_alloc(); - state->args = args; - state->mp = args->dp->i_mount; + state = xfs_da_state_alloc(args); + /* * Fill in the path to the entry in the cursor. */ @@ -2139,9 +2136,7 @@ xfs_dir2_node_removename( /* * Allocate and initialize the btree cursor. */ - state = xfs_da_state_alloc(); - state->args = args; - state->mp = args->dp->i_mount; + state = xfs_da_state_alloc(args); /* Look up the entry we're deleting, set up the cursor. */ error = xfs_da3_node_lookup_int(state, &rval); @@ -2206,9 +2201,7 @@ xfs_dir2_node_replace( /* * Allocate and initialize the btree cursor. */ - state = xfs_da_state_alloc(); - state->args = args; - state->mp = args->dp->i_mount; + state = xfs_da_state_alloc(args); /* * We have to save new inode number and ftype since diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c index 44b15015021f3..e56786f0a13c8 100644 --- a/fs/xfs/scrub/dabtree.c +++ b/fs/xfs/scrub/dabtree.c @@ -476,9 +476,7 @@ xchk_da_btree( ds.dargs.whichfork = whichfork; ds.dargs.trans = sc->tp; ds.dargs.op_flags = XFS_DA_OP_OKNOENT; - ds.state = xfs_da_state_alloc(); - ds.state->args = &ds.dargs; - ds.state->mp = mp; + ds.state = xfs_da_state_alloc(&ds.dargs); ds.sc = sc; ds.private = private; if (whichfork == XFS_ATTR_FORK) {
Every call to xfs_da_state_alloc() also requires setting up state->args and state->mp Change xfs_da_state_alloc() to receive an xfs_da_args_t as argument and return a xfs_da_state_t with both args and mp already set. Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> --- Changelog: V3: - Originally this patch removed xfs_da_state_alloc(), per hch's suggestion, instead of removing, it has been refactored, to also set state->{args,mp} which removes a few lines of code. fs/xfs/libxfs/xfs_attr.c | 17 +++++------------ fs/xfs/libxfs/xfs_da_btree.c | 8 ++++++-- fs/xfs/libxfs/xfs_da_btree.h | 2 +- fs/xfs/libxfs/xfs_dir2_node.c | 17 +++++------------ fs/xfs/scrub/dabtree.c | 4 +--- 5 files changed, 18 insertions(+), 30 deletions(-)