Re: [patch] voluntary-preempt-2.6.8-rc2-J4

Jens Axboe
2004-07-27T08:21:36Z

On Tue, Jul 27 2004, Ingo Molnar wrote:
>
> [i've sent a second patch too since the first version.]
>
> * Jens Axboe <axboe@suse.de> wrote:
>
> > I don't like it. First of all, the implementation really should drain
> > the queue first, then set max value before allowing people to queue
> > more io. The queue lock doesn't help here, readers don't even attempt
> > to serialize access to max_sectors.
>
> why should the queue be drained? There might be a few leftover big
> requests, but these are not a problem.

The patch I commented on did not split sectors value into two, in that
case it's a big problem. When split and expose only the one, it's not an
issue.

> > Secondly, I don't like the concept of exposing this value. If you want
> > to do something like this, we must split the value into two like
> > proposed (and patched) some months ago into a hardware and user value.
>
> yes, agreed - that's what the second patch does.

Great.

> > I don't see why we can't just drop ata48 default value to 256kb
> > instead. There's very little command over head on ide, I bet the
> > majority of the change in performance when playing with 256kb vs
> > 1024kb is not the command overhead itself, rather things like
> > read-ahead that could be more intelligent.
>
> 256kb isnt enough from a latency POV either - and if a user wants some
> extreme setting like 16KB per request why not allow it? Especially since
> these tunables cause zero runtime overhead.

Note that I've been travelling and didn't really track the thread
closely before that, but it seems to me that most of the latency
incurred is an artifact of long code paths hanging off bi_end_io. Maybe
it would be better to try and fix some of that and get both good irq
completion latencies, while still maintaining decent drive throughput.

That said, this second version looks better from an inclusion point of
view. Better fix those manual q->max_sectors settings first.

--
Jens Axboe