On Sun, Aug 21, 2016 at 11:51:12PM +0200, Christoph Hellwig wrote:
> For long growing file writes we will usually already have the eofblocks
> tag set when adding more speculative preallocations. Add a flag in the
> inode to allow us to skip the the fairly expensive AG-wide spinlocks
> and multiple radix tree operations in that case.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/xfs_icache.c | 14 ++++++++++++++
> fs/xfs/xfs_inode.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
> index fb39a66..65b2e3f 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -1414,6 +1414,16 @@ xfs_inode_set_eofblocks_tag(
> struct xfs_perag *pag;
> int tagged;
>
> + /*
> + * Don't bother locking the AG and looking up in the radix trees
> + * if we already know that we have the tag set.
> + */
> + if (ip->i_flags & XFS_IEOFBLOCKS)
> + return;
> + spin_lock(&ip->i_flags_lock);
> + ip->i_flags |= XFS_IEOFBLOCKS;
> + spin_unlock(&ip->i_flags_lock);
> +
I'm guessing the lockless check is intentional, but is that really
necessary? E.g., it doesn't seem like using ->i_flags_lock
unconditionally should affect performance in the way the AG lock or
radix tree work does, particularly since we're already holding
IOLOCK_EXCL in the current implementation. I could be wrong, but FWIW,
we do already have xfs_iflags_test_and_set() sitting around as well...
Brian
> pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
> spin_lock(&pag->pag_ici_lock);
> trace_xfs_inode_set_eofblocks_tag(ip);
> @@ -1449,6 +1459,10 @@ xfs_inode_clear_eofblocks_tag(
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_perag *pag;
>
> + spin_lock(&ip->i_flags_lock);
> + ip->i_flags &= ~XFS_IEOFBLOCKS;
> + spin_unlock(&ip->i_flags_lock);
> +
> pag = xfs_perag_get(mp, XFS_INO_TO_AGNO(mp, ip->i_ino));
> spin_lock(&pag->pag_ici_lock);
> trace_xfs_inode_clear_eofblocks_tag(ip);
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index e1a411e..8f30d25 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -216,6 +216,7 @@ xfs_get_initial_prid(struct xfs_inode *dp)
> #define __XFS_IPINNED_BIT 8 /* wakeup key for zero pin count */
> #define XFS_IPINNED (1 << __XFS_IPINNED_BIT)
> #define XFS_IDONTCACHE (1 << 9) /* don't cache the inode long
> term */
> +#define XFS_IEOFBLOCKS (1 << 10)/* has the preallocblocks tag
> set */
>
> /*
> * Per-lifetime flags need to be reset when re-using a reclaimable inode
> during
> --
> 2.1.4
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
|