On Wed, May 20, 2020 at 08:23:09AM +1000, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> Seeing massive cpu usage from xfs_agino_range() on one machine;
> instruction level profiles look similar to another machine running
> the same workload, only one machien is consuming 10x as much CPU as
> the other and going much slower. The only real difference between
> the two machines is core count per socket. Both are running
> identical 16p/16GB virtual machine configurations
>
> Machine A:
>
> 25.83% [k] xfs_agino_range
> 12.68% [k] __xfs_dir3_data_check
> 6.95% [k] xfs_verify_ino
> 6.78% [k] xfs_dir2_data_entry_tag_p
> 3.56% [k] xfs_buf_find
> 2.31% [k] xfs_verify_dir_ino
> 2.02% [k] xfs_dabuf_map.constprop.0
> 1.65% [k] xfs_ag_block_count
>
> And takes around 13 minutes to remove 50 million inodes.
>
> Machine B:
>
> 13.90% [k] __pv_queued_spin_lock_slowpath
> 3.76% [k] do_raw_spin_lock
> 2.83% [k] xfs_dir3_leaf_check_int
> 2.75% [k] xfs_agino_range
> 2.51% [k] __raw_callee_save___pv_queued_spin_unlock
> 2.18% [k] __xfs_dir3_data_check
> 2.02% [k] xfs_log_commit_cil
>
> And takes around 5m30s to remove 50 million inodes.
>
> Suspect is cacheline contention on m_sectbb_log which is used in one
> of the macros in xfs_agino_range. This is a read-only variable but
> shares a cacheline with m_active_trans which is a global atomic that
> gets bounced all around the machine.
>
> The workload is trying to run hundreds of thousands of transactions
> per second and hence cacheline contention will be occuring on this
> atomic counter. Hence xfs_agino_range() is likely just be an
> innocent bystander as the cache coherency protocol fights over the
> cacheline between CPU cores and sockets.
>
> On machine A, this rearrangement of the struct xfs_mount
> results in the profile changing to:
>
> 9.77% [kernel] [k] xfs_agino_range
> 6.27% [kernel] [k] __xfs_dir3_data_check
> 5.31% [kernel] [k] __pv_queued_spin_lock_slowpath
> 4.54% [kernel] [k] xfs_buf_find
> 3.79% [kernel] [k] do_raw_spin_lock
> 3.39% [kernel] [k] xfs_verify_ino
> 2.73% [kernel] [k] __raw_callee_save___pv_queued_spin_unlock
>
> Vastly less CPU usage in xfs_agino_range(), but still 3x the amount
> of machine B and still runs substantially slower than it should.
>
> Current rm -rf of 50 million files:
>
> vanilla patched
> machine A 13m20s 6m42s
> machine B 5m30s 5m02s
>
> It's an improvement, hence indicating that separation and further
> optimisation of read-only global filesystem data is worthwhile, but
> it clearly isn't the underlying issue causing this specific
> performance degradation.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
--D
> ---
> fs/xfs/xfs_mount.h | 148 +++++++++++++++++++++++++--------------------
> 1 file changed, 82 insertions(+), 66 deletions(-)
>
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 4835581f3eb00..c1f92c1847bb2 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -55,61 +55,25 @@ struct xfs_error_cfg {
> long retry_timeout; /* in jiffies, -1 = infinite */
> };
>
> +/*
> + * The struct xfsmount layout is optimised to separate read-mostly variables
> + * from variables that are frequently modified. We put the read-mostly variables
> + * first, then place all the other variables at the end.
> + *
> + * Typically, read-mostly variables are those that are set at mount time and
> + * never changed again, or only change rarely as a result of things like sysfs
> + * knobs being tweaked.
> + */
> typedef struct xfs_mount {
> + struct xfs_sb m_sb; /* copy of fs superblock */
> struct super_block *m_super;
> -
> - /*
> - * Bitsets of per-fs metadata that have been checked and/or are sick.
> - * Callers must hold m_sb_lock to access these two fields.
> - */
> - uint8_t m_fs_checked;
> - uint8_t m_fs_sick;
> - /*
> - * Bitsets of rt metadata that have been checked and/or are sick.
> - * Callers must hold m_sb_lock to access this field.
> - */
> - uint8_t m_rt_checked;
> - uint8_t m_rt_sick;
> -
> struct xfs_ail *m_ail; /* fs active log item list */
> -
> - struct xfs_sb m_sb; /* copy of fs superblock */
> - spinlock_t m_sb_lock; /* sb counter lock */
> - struct percpu_counter m_icount; /* allocated inodes counter */
> - struct percpu_counter m_ifree; /* free inodes counter */
> - struct percpu_counter m_fdblocks; /* free block counter */
> - /*
> - * Count of data device blocks reserved for delayed allocations,
> - * including indlen blocks. Does not include allocated CoW staging
> - * extents or anything related to the rt device.
> - */
> - struct percpu_counter m_delalloc_blks;
> -
> struct xfs_buf *m_sb_bp; /* buffer for superblock */
> char *m_rtname; /* realtime device name */
> char *m_logname; /* external log device name */
> - int m_bsize; /* fs logical block size */
> - xfs_agnumber_t m_agfrotor; /* last ag where space found */
> - xfs_agnumber_t m_agirotor; /* last ag dir inode alloced */
> - spinlock_t m_agirotor_lock;/* .. and lock protecting it */
> - xfs_agnumber_t m_maxagi; /* highest inode alloc group */
> - uint m_allocsize_log;/* min write size log bytes */
> - uint m_allocsize_blocks; /* min write size blocks */
> struct xfs_da_geometry *m_dir_geo; /* directory block geometry */
> struct xfs_da_geometry *m_attr_geo; /* attribute block geometry */
> struct xlog *m_log; /* log specific stuff */
> - struct xfs_ino_geometry m_ino_geo; /* inode geometry */
> - int m_logbufs; /* number of log buffers */
> - int m_logbsize; /* size of each log buffer */
> - uint m_rsumlevels; /* rt summary levels */
> - uint m_rsumsize; /* size of rt summary, bytes */
> - /*
> - * Optional cache of rt summary level per bitmap block with the
> - * invariant that m_rsum_cache[bbno] <= the minimum i for which
> - * rsum[i][bbno] != 0. Reads and writes are serialized by the rsumip
> - * inode lock.
> - */
> - uint8_t *m_rsum_cache;
> struct xfs_inode *m_rbmip; /* pointer to bitmap inode */
> struct xfs_inode *m_rsumip; /* pointer to summary inode */
> struct xfs_inode *m_rootip; /* pointer to root directory */
> @@ -117,9 +81,26 @@ typedef struct xfs_mount {
> xfs_buftarg_t *m_ddev_targp; /* saves taking the address */
> xfs_buftarg_t *m_logdev_targp;/* ptr to log device */
> xfs_buftarg_t *m_rtdev_targp; /* ptr to rt device */
> + /*
> + * Optional cache of rt summary level per bitmap block with the
> + * invariant that m_rsum_cache[bbno] <= the minimum i for which
> + * rsum[i][bbno] != 0. Reads and writes are serialized by the rsumip
> + * inode lock.
> + */
> + uint8_t *m_rsum_cache;
> + struct xfs_mru_cache *m_filestream; /* per-mount filestream data */
> + struct workqueue_struct *m_buf_workqueue;
> + struct workqueue_struct *m_unwritten_workqueue;
> + struct workqueue_struct *m_cil_workqueue;
> + struct workqueue_struct *m_reclaim_workqueue;
> + struct workqueue_struct *m_eofblocks_workqueue;
> + struct workqueue_struct *m_sync_workqueue;
> +
> + int m_bsize; /* fs logical block size */
> uint8_t m_blkbit_log; /* blocklog + NBBY */
> uint8_t m_blkbb_log; /* blocklog - BBSHIFT */
> uint8_t m_agno_log; /* log #ag's */
> + uint8_t m_sectbb_log; /* sectlog - BBSHIFT */
> uint m_blockmask; /* sb_blocksize-1 */
> uint m_blockwsize; /* sb_blocksize in words */
> uint m_blockwmask; /* blockwsize-1 */
> @@ -138,47 +119,83 @@ typedef struct xfs_mount {
> xfs_extlen_t m_ag_prealloc_blocks; /* reserved ag blocks */
> uint m_alloc_set_aside; /* space we can't use */
> uint m_ag_max_usable; /* max space per AG */
> - struct radix_tree_root m_perag_tree; /* per-ag accounting info */
> - spinlock_t m_perag_lock; /* lock for m_perag_tree */
> - struct mutex m_growlock; /* growfs mutex */
> + int m_dalign; /* stripe unit */
> + int m_swidth; /* stripe width */
> + xfs_agnumber_t m_maxagi; /* highest inode alloc group */
> + uint m_allocsize_log;/* min write size log bytes */
> + uint m_allocsize_blocks; /* min write size blocks */
> + int m_logbufs; /* number of log buffers */
> + int m_logbsize; /* size of each log buffer */
> + uint m_rsumlevels; /* rt summary levels */
> + uint m_rsumsize; /* size of rt summary, bytes */
> int m_fixedfsid[2]; /* unchanged for life of FS */
> - uint64_t m_flags; /* global mount flags */
> - bool m_finobt_nores; /* no per-AG finobt resv. */
> uint m_qflags; /* quota status flags */
> + uint64_t m_flags; /* global mount flags */
> + int64_t m_low_space[XFS_LOWSP_MAX];
> + struct xfs_ino_geometry m_ino_geo; /* inode geometry */
> struct xfs_trans_resv m_resv; /* precomputed res values */
> + /* low free space thresholds */
> + bool m_always_cow;
> + bool m_fail_unmount;
> + bool m_finobt_nores; /* no per-AG finobt resv. */
> + bool m_update_sb; /* sb needs update in mount */
> +
> + /*
> + * Bitsets of per-fs metadata that have been checked and/or are sick.
> + * Callers must hold m_sb_lock to access these two fields.
> + */
> + uint8_t m_fs_checked;
> + uint8_t m_fs_sick;
> + /*
> + * Bitsets of rt metadata that have been checked and/or are sick.
> + * Callers must hold m_sb_lock to access this field.
> + */
> + uint8_t m_rt_checked;
> + uint8_t m_rt_sick;
> +
> + /*
> + * End of read-mostly variables. Frequently written variables and locks
> + * should be placed below this comment from now on. The first variable
> + * here is marked as cacheline aligned so they it is separated from
> + * the read-mostly variables.
> + */
> +
> + spinlock_t ____cacheline_aligned m_sb_lock; /* sb counter lock */
> + struct percpu_counter m_icount; /* allocated inodes counter */
> + struct percpu_counter m_ifree; /* free inodes counter */
> + struct percpu_counter m_fdblocks; /* free block counter */
> + /*
> + * Count of data device blocks reserved for delayed allocations,
> + * including indlen blocks. Does not include allocated CoW staging
> + * extents or anything related to the rt device.
> + */
> + struct percpu_counter m_delalloc_blks;
> +
> + struct radix_tree_root m_perag_tree; /* per-ag accounting info */
> + spinlock_t m_perag_lock; /* lock for m_perag_tree */
> uint64_t m_resblks; /* total reserved blocks */
> uint64_t m_resblks_avail;/* available reserved blocks */
> uint64_t m_resblks_save; /* reserved blks @ remount,ro */
> - int m_dalign; /* stripe unit */
> - int m_swidth; /* stripe width */
> - uint8_t m_sectbb_log; /* sectlog - BBSHIFT */
> atomic_t m_active_trans; /* number trans frozen */
> - struct xfs_mru_cache *m_filestream; /* per-mount filestream data */
> struct delayed_work m_reclaim_work; /* background inode reclaim */
> struct delayed_work m_eofblocks_work; /* background eof blocks
> trimming */
> struct delayed_work m_cowblocks_work; /* background cow blocks
> trimming */
> - bool m_update_sb; /* sb needs update in mount */
> - int64_t m_low_space[XFS_LOWSP_MAX];
> - /* low free space thresholds */
> struct xfs_kobj m_kobj;
> struct xfs_kobj m_error_kobj;
> struct xfs_kobj m_error_meta_kobj;
> struct xfs_error_cfg m_error_cfg[XFS_ERR_CLASS_MAX][XFS_ERR_ERRNO_MAX];
> struct xstats m_stats; /* per-fs stats */
> + xfs_agnumber_t m_agfrotor; /* last ag where space found */
> + xfs_agnumber_t m_agirotor; /* last ag dir inode alloced */
> + spinlock_t m_agirotor_lock;/* .. and lock protecting it */
>
> /*
> * Workqueue item so that we can coalesce multiple inode flush attempts
> * into a single flush.
> */
> struct work_struct m_flush_inodes_work;
> - struct workqueue_struct *m_buf_workqueue;
> - struct workqueue_struct *m_unwritten_workqueue;
> - struct workqueue_struct *m_cil_workqueue;
> - struct workqueue_struct *m_reclaim_workqueue;
> - struct workqueue_struct *m_eofblocks_workqueue;
> - struct workqueue_struct *m_sync_workqueue;
>
> /*
> * Generation of the filesysyem layout. This is incremented by each
> @@ -190,9 +207,8 @@ typedef struct xfs_mount {
> * to various other kinds of pain inflicted on the pNFS server.
> */
> uint32_t m_generation;
> + struct mutex m_growlock; /* growfs mutex */
>
> - bool m_always_cow;
> - bool m_fail_unmount;
> #ifdef DEBUG
> /*
> * Frequency with which errors are injected. Replaces xfs_etest; the
> --
> 2.26.2.761.g0e0b3e54be
>
