Re: [patch] Prevent excessive xfsaild wakeups

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

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)) {
                ...
        }