On Sun, Mar 29, 2020 at 10:22:09AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> A customer reported rcu stalls and softlockup warnings on a computer
> with many CPU cores and many many more IO threads trying to write to a
> filesystem that is totally out of space. Subsequent analysis pointed to
> the many many IO threads calling xfs_flush_inodes -> sync_inodes_sb,
> which causes a lot of wb_writeback_work to be queued. The writeback
> worker spends so much time trying to wake the many many threads waiting
> for writeback completion that it trips the softlockup detector, and (in
> this case) the system automatically reboots.
>
> In addition, they complain that the lengthy xfs_flush_inodes scan traps
> all of those threads in uninterruptible sleep, which hampers their
> ability to kill the program or do anything else to escape the situation.
>
> If there's thousands of threads trying to write to files on a full
> filesystem, each of those threads will start separate copies of the
> inode flush scan. This is kind of pointless since we only need one
> scan, so rate limit the inode flush.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_mount.h | 1 +
> fs/xfs/xfs_super.c | 14 ++++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 88ab09ed29e7..50c43422fa17 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -167,6 +167,7 @@ typedef struct xfs_mount {
> 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 */
> + struct ratelimit_state m_flush_inodes_ratelimit;
>
> struct workqueue_struct *m_buf_workqueue;
> struct workqueue_struct *m_unwritten_workqueue;
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 68fea439d974..abf06bf9c3f3 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -528,6 +528,9 @@ xfs_flush_inodes(
> {
> struct super_block *sb = mp->m_super;
>
> + if (!__ratelimit(&mp->m_flush_inodes_ratelimit))
> + return;
> +
> if (down_read_trylock(&sb->s_umount)) {
> sync_inodes_sb(sb);
> up_read(&sb->s_umount);
> @@ -1366,6 +1369,17 @@ xfs_fc_fill_super(
> if (error)
> goto out_free_names;
>
> + /*
> + * Cap the number of invocations of xfs_flush_inodes to 16 for every
> + * quarter of a second. The magic numbers here were determined by
> + * observation neither to cause stalls in writeback when there are a
> + * lot of IO threads and the fs is near ENOSPC, nor cause any fstest
> + * regressions. YMMV.
> + */
> + ratelimit_state_init(&mp->m_flush_inodes_ratelimit, HZ / 4, 16);
> + ratelimit_set_flags(&mp->m_flush_inodes_ratelimit,
> + RATELIMIT_MSG_ON_RELEASE);
Urk.
RATELIMIT_MSG_ON_RELEASE prevents "callbacks suppressed"
messages when rate limiting was active and resets via __rate_limit().
However, in ratelimit_state_exit(), that flag -enables- printing
"callbacks suppressed" messages when rate limiting was active and is
reset.
Same flag, exact opposite behaviour...
The comment says it's behaviour is supposed to match that of
ratelimit_state_exit() (i.e. print message on ratelimit exit), so I
really can't tell if this is correct/intended usage or just API
abuse....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
