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
next prev parent 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
-- strict thread matches above, loose matches on Subject: below --
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).