Re: [PATCH 05/12] xfs: ratelimit unmount time per-buffer I/O error warning

[Dave Chinner <david@fromorbit.com>]

On Fri, Apr 17, 2020 at 11:08:52AM -0400, Brian Foster wrote:
> At unmount time, XFS emits a warning for every in-core buffer that
> might have undergone a write error. In practice this behavior is
> probably reasonable given that the filesystem is likely short lived
> once I/O errors begin to occur consistently. Under certain test or
> otherwise expected error conditions, this can spam the logs and slow
> down the unmount. Ratelimit the warning to prevent this problem
> while still informing the user that errors have occurred.
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/xfs/xfs_buf.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 93942d8e35dd..5120fed06075 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1685,11 +1685,10 @@ xfs_wait_buftarg(
>  			bp = list_first_entry(&dispose, struct xfs_buf, b_lru);
>  			list_del_init(&bp->b_lru);
>  			if (bp->b_flags & XBF_WRITE_FAIL) {
> -				xfs_alert(btp->bt_mount,
> -"Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!",
> +				xfs_alert_ratelimited(btp->bt_mount,
> +"Corruption Alert: Buffer at daddr 0x%llx had permanent write failures!\n"
> +"Please run xfs_repair to determine the extent of the problem.",
>  					(long long)bp->b_bn);

Hmmmm. I was under the impression that multiple line log messages
were frowned upon because they prevent every output line in the log
being tagged correctly. That's where KERN_CONT came from (i.e. it's
a continuation of a previous log message), but we don't use that
with the XFS logging and hence multi-line log messages are split
into multiple logging calls.

IOWs, this might be better handled just using a static ratelimit
variable here....

Actually, we already have one for xfs_buf_item_push() to limit
warnings about retrying XBF_WRITE_FAIL buffers:

static DEFINE_RATELIMIT_STATE(xfs_buf_write_fail_rl_state, 30 * HZ, 10);

Perhaps we should be using the same ratelimit variable here....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com