Re: [PATCH 8/9] libxfs: don't repeatedly shake unwritable buffers

[Brian Foster <bfoster@redhat.com>]

On Tue, Dec 22, 2015 at 08:37:08AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> now that we try to write dirty buffers before we release them, we
> can get buildup of unwritable dirty buffers on the LRU lists, This
> results in the cache shaker repeatedly trying to write out these
> buffers every time the cache fills up. This results in more
> corruption warnings, and takes up a lot of time doing reclaiming
> nothing. This can effectively livelock the processing parts of phase
> 4.
> 
> Fix this by not trying to write buffers with corruption errors on
> them. These errors will get cleared when the buffer is re-read and
> fixed and them marked dirty again. At which point, we'll be able to
> write them and so the cache can reclaim them successfully.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  libxfs/rdwr.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 0337a21..a1f0029 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -1103,7 +1103,6 @@ int
>  libxfs_writebufr(xfs_buf_t *bp)
>  {
>  	int	fd = libxfs_device_to_fd(bp->b_target->dev);
> -	int	error = 0;
>  
>  	/*
>  	 * we never write buffers that are marked stale. This indicates they
> @@ -1134,7 +1133,7 @@ libxfs_writebufr(xfs_buf_t *bp)
>  	}
>  
>  	if (!(bp->b_flags & LIBXFS_B_DISCONTIG)) {
> -		error = __write_buf(fd, bp->b_addr, bp->b_bcount,
> +		bp->b_error = __write_buf(fd, bp->b_addr, bp->b_bcount,
>  				    LIBXFS_BBTOOFF64(bp->b_bn), bp->b_flags);
>  	} else {
>  		int	i;
> @@ -1144,11 +1143,10 @@ libxfs_writebufr(xfs_buf_t *bp)
>  			off64_t	offset = LIBXFS_BBTOOFF64(bp->b_map[i].bm_bn);
>  			int len = BBTOB(bp->b_map[i].bm_len);
>  
> -			error = __write_buf(fd, buf, len, offset, bp->b_flags);
> -			if (error) {
> -				bp->b_error = error;
> +			bp->b_error = __write_buf(fd, buf, len, offset,
> +						  bp->b_flags);
> +			if (bp->b_error)
>  				break;
> -			}
>  			buf += len;
>  		}
>  	}
> @@ -1157,14 +1155,14 @@ libxfs_writebufr(xfs_buf_t *bp)
>  	printf("%lx: %s: wrote %u bytes, blkno=%llu(%llu), %p, error %d\n",
>  			pthread_self(), __FUNCTION__, bp->b_bcount,
>  			(long long)LIBXFS_BBTOOFF64(bp->b_bn),
> -			(long long)bp->b_bn, bp, error);
> +			(long long)bp->b_bn, bp, bp->b_error);
>  #endif
> -	if (!error) {
> +	if (!bp->b_error) {
>  		bp->b_flags |= LIBXFS_B_UPTODATE;
>  		bp->b_flags &= ~(LIBXFS_B_DIRTY | LIBXFS_B_EXIT |
>  				 LIBXFS_B_UNCHECKED);
>  	}
> -	return error;
> +	return bp->b_error;
>  }
>  
>  int
> @@ -1266,15 +1264,22 @@ libxfs_bulkrelse(
>  	return count;
>  }
>  
> +/*
> + * When a buffer is marked dirty, the error is cleared. Hence if we are trying
> + * to flush a buffer prior to cache reclaim that has an error on it it means
> + * we've already tried to flush it and it failed. Prevent repeated corruption
> + * errors from being reported by skipping such buffers - when the corruption is
> + * fixed the buffer will be marked dirty again and we can write it again.
> + */
>  static int
>  libxfs_bflush(
>  	struct cache_node	*node)
>  {
>  	struct xfs_buf		*bp = (struct xfs_buf *)node;
>  
> -	if (bp->b_flags & LIBXFS_B_DIRTY)
> +	if (!bp->b_error && bp->b_flags & LIBXFS_B_DIRTY)
>  		return libxfs_writebufr(bp);
> -	return 0;
> +	return bp->b_error;
>  }
>  
>  void
> -- 
> 2.5.0
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs