From: Brian Foster <bfoster@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org, Chandan Babu R <chandanrlinux@gmail.com>
Subject: Re: [PATCH 6/6] xfs: cleanup xfs_idestroy_fork
Date: Mon, 18 May 2020 09:28:16 -0400 [thread overview]
Message-ID: <20200518132816.GE10938@bfoster> (raw)
In-Reply-To: <20200518073358.760214-7-hch@lst.de>
On Mon, May 18, 2020 at 09:33:58AM +0200, Christoph Hellwig wrote:
> Move freeing the dynamically allocated attr and COW fork, as well
> as zeroing the pointers where actually needed into the callers, and
> just pass the xfs_ifork structure to xfs_idestroy_fork. Also simplify
> the kmem_free calls by not checking for NULL first.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> fs/xfs/libxfs/xfs_attr_leaf.c | 7 +++----
> fs/xfs/libxfs/xfs_inode_buf.c | 2 +-
> fs/xfs/libxfs/xfs_inode_fork.c | 32 +++++++++-----------------------
> fs/xfs/libxfs/xfs_inode_fork.h | 2 +-
> fs/xfs/xfs_attr_inactive.c | 7 +++++--
> fs/xfs/xfs_icache.c | 15 +++++++++------
> 6 files changed, 28 insertions(+), 37 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index b279200519777..394805abb4535 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -718,11 +718,10 @@ xfs_attr_fork_remove(
> {
> ASSERT(ip->i_afp->if_nextents == 0);
>
> - xfs_idestroy_fork(ip, XFS_ATTR_FORK);
> + xfs_idestroy_fork(ip->i_afp);
> + kmem_cache_free(xfs_ifork_zone, ip->i_afp);
> + ip->i_afp = NULL;
> ip->i_d.di_forkoff = 0;
> -
> - ASSERT(ip->i_afp == NULL);
> -
> xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> }
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index ab555671e1543..6f84ea85fdd83 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -271,7 +271,7 @@ xfs_inode_from_disk(
> return 0;
>
> out_destroy_data_fork:
> - xfs_idestroy_fork(ip, XFS_DATA_FORK);
> + xfs_idestroy_fork(&ip->i_df);
> return error;
> }
>
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index ef43b4893766c..28b366275ae0e 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -503,38 +503,24 @@ xfs_idata_realloc(
>
> void
> xfs_idestroy_fork(
> - xfs_inode_t *ip,
> - int whichfork)
> + struct xfs_ifork *ifp)
> {
> - struct xfs_ifork *ifp;
> -
> - ifp = XFS_IFORK_PTR(ip, whichfork);
> if (ifp->if_broot != NULL) {
> kmem_free(ifp->if_broot);
> ifp->if_broot = NULL;
> }
>
> /*
> - * If the format is local, then we can't have an extents
> - * array so just look for an inline data array. If we're
> - * not local then we may or may not have an extents list,
> - * so check and free it up if we do.
> + * If the format is local, then we can't have an extents array so just
> + * look for an inline data array. If we're not local then we may or may
> + * not have an extents list, so check and free it up if we do.
> */
> if (ifp->if_format == XFS_DINODE_FMT_LOCAL) {
> - if (ifp->if_u1.if_data != NULL) {
> - kmem_free(ifp->if_u1.if_data);
> - ifp->if_u1.if_data = NULL;
> - }
> - } else if ((ifp->if_flags & XFS_IFEXTENTS) && ifp->if_height) {
> - xfs_iext_destroy(ifp);
> - }
> -
> - if (whichfork == XFS_ATTR_FORK) {
> - kmem_cache_free(xfs_ifork_zone, ip->i_afp);
> - ip->i_afp = NULL;
> - } else if (whichfork == XFS_COW_FORK) {
> - kmem_cache_free(xfs_ifork_zone, ip->i_cowfp);
> - ip->i_cowfp = NULL;
> + kmem_free(ifp->if_u1.if_data);
> + ifp->if_u1.if_data = NULL;
> + } else if (ifp->if_flags & XFS_IFEXTENTS) {
> + if (ifp->if_height)
> + xfs_iext_destroy(ifp);
> }
> }
>
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index d849cca103edd..a4953e95c4f3f 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -86,7 +86,7 @@ int xfs_iformat_data_fork(struct xfs_inode *, struct xfs_dinode *);
> int xfs_iformat_attr_fork(struct xfs_inode *, struct xfs_dinode *);
> void xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
> struct xfs_inode_log_item *, int);
> -void xfs_idestroy_fork(struct xfs_inode *, int);
> +void xfs_idestroy_fork(struct xfs_ifork *ifp);
> void xfs_idata_realloc(struct xfs_inode *ip, int64_t byte_diff,
> int whichfork);
> void xfs_iroot_realloc(struct xfs_inode *, int, int);
> diff --git a/fs/xfs/xfs_attr_inactive.c b/fs/xfs/xfs_attr_inactive.c
> index 00ffc46c0bf71..bfad669e6b2f8 100644
> --- a/fs/xfs/xfs_attr_inactive.c
> +++ b/fs/xfs/xfs_attr_inactive.c
> @@ -388,8 +388,11 @@ xfs_attr_inactive(
> xfs_trans_cancel(trans);
> out_destroy_fork:
> /* kill the in-core attr fork before we drop the inode lock */
> - if (dp->i_afp)
> - xfs_idestroy_fork(dp, XFS_ATTR_FORK);
> + if (dp->i_afp) {
> + xfs_idestroy_fork(dp->i_afp);
> + kmem_cache_free(xfs_ifork_zone, dp->i_afp);
> + dp->i_afp = NULL;
> + }
> if (lock_mode)
> xfs_iunlock(dp, lock_mode);
> return error;
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index c09b3e9eab1da..d806d3bfa8936 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -87,15 +87,18 @@ xfs_inode_free_callback(
> case S_IFREG:
> case S_IFDIR:
> case S_IFLNK:
> - xfs_idestroy_fork(ip, XFS_DATA_FORK);
> + xfs_idestroy_fork(&ip->i_df);
> break;
> }
>
> - if (ip->i_afp)
> - xfs_idestroy_fork(ip, XFS_ATTR_FORK);
> - if (ip->i_cowfp)
> - xfs_idestroy_fork(ip, XFS_COW_FORK);
> -
> + if (ip->i_afp) {
> + xfs_idestroy_fork(ip->i_afp);
> + kmem_cache_free(xfs_ifork_zone, ip->i_afp);
> + }
> + if (ip->i_cowfp) {
> + xfs_idestroy_fork(ip->i_cowfp);
> + kmem_cache_free(xfs_ifork_zone, ip->i_cowfp);
> + }
> if (ip->i_itemp) {
> ASSERT(!test_bit(XFS_LI_IN_AIL,
> &ip->i_itemp->ili_item.li_flags));
> --
> 2.26.2
>
next prev parent reply other threads:[~2020-05-18 13:28 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-18 7:33 move the extent count and format into struct xfs_ifork v2 Christoph Hellwig
2020-05-18 7:33 ` [PATCH 1/6] xfs: clean up xchk_bmap_check_rmaps usage of XFS_IFORK_Q Christoph Hellwig
2020-05-18 13:28 ` Brian Foster
2020-05-18 7:33 ` [PATCH 2/6] xfs: remove the XFS_DFORK_Q macro Christoph Hellwig
2020-05-18 7:33 ` [PATCH 3/6] xfs: remove xfs_ifree_local_data Christoph Hellwig
2020-05-18 7:33 ` [PATCH 4/6] xfs: move the per-fork nextents fields into struct xfs_ifork Christoph Hellwig
2020-05-18 7:33 ` [PATCH 5/6] xfs: move the fork format " Christoph Hellwig
2020-05-18 7:33 ` [PATCH 6/6] xfs: cleanup xfs_idestroy_fork Christoph Hellwig
2020-05-18 13:28 ` Brian Foster [this message]
-- strict thread matches above, loose matches on Subject: below --
2020-05-10 7:23 move the extent count and format into struct xfs_ifork Christoph Hellwig
2020-05-10 7:24 ` [PATCH 6/6] xfs: cleanup xfs_idestroy_fork Christoph Hellwig
2020-05-12 9:48 ` Chandan Babu R
2020-05-12 18:54 ` Brian Foster
2020-05-16 18:10 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200518132816.GE10938@bfoster \
--to=bfoster@redhat.com \
--cc=chandanrlinux@gmail.com \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.