Re: [PATCH][CFT] new IO scheduler for 2.4.20

[Andrew Morton <akpm@digeo.com>]

Neil Schemenauer <nas@arctrix.com> wrote:
>
> Hi all,
> 
> Recently I was bitten badly by bad IO scheduler behavior on an important
> Linux server.  An easy way to trigger this problem is to start a
> streaming write process:
> 
>     while :
>     do
>             dd if=/dev/zero of=foo bs=1M count=512 conv=notrunc
>     done

That's a local DoS.

> and then try doing a bunch of small reads:
> 
>     time (find kernel-tree -type f | xargs cat > /dev/null)

Awful, isn't it?

> +int elevator_neil_merge(request_queue_t *q, struct request **req,

This is a nice looking patch!

> ...
> +	unsigned int expire_time = jiffies - 1*HZ;
> ...
> +		if (time_before(__rq->start_time, expire_time)) {
> +			break;
> +		}

It has a deadline component.  One second is probably too long.

> +	if (!*req && rw == READ) {
> +		long extra_writes = write_sectors - read_sectors;
> +		/*
> +		 * If there are more writes than reads in the queue then put
> +		 * read requests ahead of the extra writes.  This prevents
> +		 * writes from starving reads.
> +		 */
> +		entry = q->queue_head.prev;
> +		while (extra_writes > 0 && entry != head) {
> +			__rq = blkdev_entry_to_request(entry);
> +			if (__rq->cmd == WRITE)
> +				extra_writes -= __rq->nr_sectors;
> +			entry = entry->prev;
> +		}
> +		*req = blkdev_entry_to_request(entry);

One suggestion I'd make here is to not count "sectors" at all.  Just count
requests.

See, the code at present is assuming that 100 discrete requests are
equivalent to a single 100 sector request.  And that is most definitely not
the case.  A 100 sector request is worth (guess) just two discontiguous
requests.


I'd be interested in seeing some comparative benchmark results with the above
DoS attack.  And contest numbers too...