Re: [patch] Prevent excessive xfsaild wakeups

[Christoph Hellwig <hch@infradead.org>]

On Tue, Feb 19, 2008 at 09:59:06AM +1100, David Chinner wrote:
> +	if (count && (XFS_LSN_CMP(lsn, target) >= 0)) {
> +		/*
> +		 * We reached the target so wait a bit longer for I/O to
> +		 * complete and remove pushed items from the AIL before we
> +		 * start the next scan from the start of the AIL.
> +		 */
>  		tout += 20;
>  		last_pushed_lsn = 0;
> +	} else if (!count) {
> +		/* We're past our target or empty, so idle */
> +		tout = 1000;
>  	} else if ((restarts > XFS_TRANS_PUSH_AIL_RESTARTS) ||
>  		   (count && ((stuck * 100) / count > 90))) {
>  		/*

When looking at this conditions it confuses the hell out of me.  Having
count checked in each of three nested conditionals simply isn't readable
:)

Also some checks seem to be superflous, so how about:


	if (!count) {
		/*
		 * We're past our target or empty, so idle.
		 */
		 tout = 1000;
	} else if (XFS_LSN_CMP(lsn, target) >= 0) {
		/*
		 * We reached the target so wait a bit longer for I/O to
		 * complete and remove pushed items from the AIL before we
		 * start the next scan from the start of the AIL.
		 */
 		tout += 20;
 		last_pushed_lsn = 0;
	} else (restarts > XFS_TRANS_PUSH_AIL_RESTARTS ||
	        ((stuck * 100) / count > 90)) {
		...
	}