All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Christoph Hellwig <hch@lst.de>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 4/6] xfs: introduce a new xfs_inode_has_cow_data helper
Date: Wed, 11 Jul 2018 09:24:34 -0700	[thread overview]
Message-ID: <20180711162434.GD32415@magnolia> (raw)
In-Reply-To: <20180710060528.4071-5-hch@lst.de>

On Tue, Jul 10, 2018 at 08:05:26AM +0200, Christoph Hellwig wrote:
> We have a few places that already check if an inode has actual data in
> the COW fork to avoid work on reflink inodes that do not actually have
> outstanding COW blocks.  There are a few more places that can avoid
> working if doing the same check, so add a documented helper for this
> condition and use it in all places where it makes sense.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok, I think.
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_aops.c      |  4 ++--
>  fs/xfs/xfs_bmap_util.c |  2 +-
>  fs/xfs/xfs_icache.c    | 10 ++++------
>  fs/xfs/xfs_inode.c     |  3 +--
>  fs/xfs/xfs_inode.h     |  9 +++++++++
>  fs/xfs/xfs_reflink.c   |  2 +-
>  6 files changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index f4d3252236c1..814100d27343 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -338,7 +338,7 @@ xfs_map_blocks(
>  	imap_valid = offset_fsb >= wpc->imap.br_startoff &&
>  		     offset_fsb < wpc->imap.br_startoff + wpc->imap.br_blockcount;
>  	if (imap_valid &&
> -	    (!xfs_is_reflink_inode(ip) || wpc->io_type == XFS_IO_COW))
> +	    (!xfs_inode_has_cow_data(ip) || wpc->io_type == XFS_IO_COW))
>  		return 0;
>  
>  	if (XFS_FORCED_SHUTDOWN(mp))
> @@ -363,7 +363,7 @@ xfs_map_blocks(
>  	 * Check if this is offset is covered by a COW extents, and if yes use
>  	 * it directly instead of looking up anything in the data fork.
>  	 */
> -	if (xfs_is_reflink_inode(ip) &&
> +	if (xfs_inode_has_cow_data(ip) &&
>  	    xfs_iext_lookup_extent(ip, ip->i_cowfp, offset_fsb, &icur, &imap) &&
>  	    imap.br_startoff <= offset_fsb) {
>  		xfs_iunlock(ip, XFS_ILOCK_SHARED);
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index da561882c349..d78f8300607f 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1280,7 +1280,7 @@ xfs_prepare_shift(
>  	 * we've flushed all the dirty data out to disk to avoid having
>  	 * CoW extents at the wrong offsets.
>  	 */
> -	if (xfs_is_reflink_inode(ip)) {
> +	if (xfs_inode_has_cow_data(ip)) {
>  		error = xfs_reflink_cancel_cow_range(ip, offset, NULLFILEOFF,
>  				true);
>  		if (error)
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index 79f344fa8b14..fdae4c2d461e 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -1697,14 +1697,13 @@ xfs_inode_clear_eofblocks_tag(
>   */
>  static bool
>  xfs_prep_free_cowblocks(
> -	struct xfs_inode	*ip,
> -	struct xfs_ifork	*ifp)
> +	struct xfs_inode	*ip)
>  {
>  	/*
>  	 * Just clear the tag if we have an empty cow fork or none at all. It's
>  	 * possible the inode was fully unshared since it was originally tagged.
>  	 */
> -	if (!xfs_is_reflink_inode(ip) || !ifp->if_bytes) {
> +	if (!xfs_inode_has_cow_data(ip)) {
>  		trace_xfs_inode_free_cowblocks_invalid(ip);
>  		xfs_inode_clear_cowblocks_tag(ip);
>  		return false;
> @@ -1742,11 +1741,10 @@ xfs_inode_free_cowblocks(
>  	void			*args)
>  {
>  	struct xfs_eofblocks	*eofb = args;
> -	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
>  	int			match;
>  	int			ret = 0;
>  
> -	if (!xfs_prep_free_cowblocks(ip, ifp))
> +	if (!xfs_prep_free_cowblocks(ip))
>  		return 0;
>  
>  	if (eofb) {
> @@ -1771,7 +1769,7 @@ xfs_inode_free_cowblocks(
>  	 * Check again, nobody else should be able to dirty blocks or change
>  	 * the reflink iflag now that we have the first two locks held.
>  	 */
> -	if (xfs_prep_free_cowblocks(ip, ifp))
> +	if (xfs_prep_free_cowblocks(ip))
>  		ret = xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, false);
>  
>  	xfs_iunlock(ip, XFS_MMAPLOCK_EXCL);
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index b6da446ae946..2036e49f7e15 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1868,7 +1868,6 @@ xfs_inactive(
>  	xfs_inode_t	*ip)
>  {
>  	struct xfs_mount	*mp;
> -	struct xfs_ifork	*cow_ifp = XFS_IFORK_PTR(ip, XFS_COW_FORK);
>  	int			error;
>  	int			truncate = 0;
>  
> @@ -1889,7 +1888,7 @@ xfs_inactive(
>  		return;
>  
>  	/* Try to clean out the cow blocks if there are any. */
> -	if (xfs_is_reflink_inode(ip) && cow_ifp->if_bytes > 0)
> +	if (xfs_inode_has_cow_data(ip))
>  		xfs_reflink_cancel_cow_range(ip, 0, NULLFILEOFF, true);
>  
>  	if (VFS_I(ip)->i_nlink != 0) {
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 1f910d2ae73a..82d27a295336 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -198,6 +198,15 @@ static inline bool xfs_is_reflink_inode(struct xfs_inode *ip)
>  	return ip->i_d.di_flags2 & XFS_DIFLAG2_REFLINK;
>  }
>  
> +/*
> + * Check if an inode has any data in the COW fork.  This might be often false
> + * even for inodes with the reflink flag when there is no pending COW operation.
> + */
> +static inline bool xfs_inode_has_cow_data(struct xfs_inode *ip)
> +{
> +	return ip->i_cowfp && ip->i_cowfp->if_bytes;
> +}
> +
>  /*
>   * In-core inode flags.
>   */
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 49e4913fa779..789310c547e6 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -494,7 +494,7 @@ xfs_reflink_cancel_cow_blocks(
>  	struct xfs_defer_ops		dfops;
>  	int				error = 0;
>  
> -	if (!xfs_is_reflink_inode(ip))
> +	if (!xfs_inode_has_cow_data(ip))
>  		return 0;
>  	if (!xfs_iext_lookup_extent_before(ip, ifp, &end_fsb, &icur, &got))
>  		return 0;
> -- 
> 2.18.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2018-07-11 16:29 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-10  6:05 reduce lookups in the COW extent tree Christoph Hellwig
2018-07-10  6:05 ` [PATCH 1/6] xfs: remove if_real_bytes Christoph Hellwig
2018-07-11 16:22   ` Darrick J. Wong
2018-07-10  6:05 ` [PATCH 2/6] xfs: simplify xfs_idata_realloc Christoph Hellwig
2018-07-11 16:23   ` Darrick J. Wong
2018-07-10  6:05 ` [PATCH 3/6] xfs: remove the xfs_ifork_t typedef Christoph Hellwig
2018-07-11 16:23   ` Darrick J. Wong
2018-07-10  6:05 ` [PATCH 4/6] xfs: introduce a new xfs_inode_has_cow_data helper Christoph Hellwig
2018-07-11 16:24   ` Darrick J. Wong [this message]
2018-07-10  6:05 ` [PATCH 5/6] xfs: maintain a sequence count for inode fork manipulations Christoph Hellwig
2018-07-10  6:05 ` [PATCH 6/6] xfs: avoid COW fork extent lookups in writeback if the fork didn't change Christoph Hellwig
2018-07-11 17:15   ` Darrick J. Wong
2018-07-11 17:20     ` Christoph Hellwig
2018-07-11 17:31       ` Darrick J. Wong
2018-07-11 17:35         ` Christoph Hellwig
2018-07-12 13:49 reduce lookups in the COW extent tree V2 Christoph Hellwig
2018-07-12 13:49 ` [PATCH 4/6] xfs: introduce a new xfs_inode_has_cow_data helper Christoph Hellwig
2018-07-17 23:23 reduce lookups in the COW extent tree V3 Christoph Hellwig
2018-07-17 23:24 ` [PATCH 4/6] xfs: introduce a new xfs_inode_has_cow_data helper Christoph Hellwig

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=20180711162434.GD32415@magnolia \
    --to=darrick.wong@oracle.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.