Re: [PATCH] speed up SATA

[Jeff Garzik <jgarzik@pobox.com>]

Andrea Arcangeli wrote:
> On Sun, Mar 28, 2004 at 01:59:51PM -0500, Jeff Garzik wrote:
> 
>>Bartlomiej Zolnierkiewicz wrote:
>>
>>>On Sunday 28 of March 2004 20:30, Jens Axboe wrote:
>>>
>>>>Making something user tunable is usually not the best idea, if you can
>>>>deduct these things automagically instead. So whether this is the best
>>>>idea, depends on which way you want to go.
>>>
>>>
>>>I think it's the best idea for now, long-term we are better with automagic.
>>
>>
>>Mostly agreed:
>>
>>Like I mentioned in the last message, the IO scheduler and the VM should 
> 
> 
> this is not an I/O scheduler or VM issue.

This involves the interaction of three:  blkdev layer, IO scheduler, and VM.

VM:  initiates most of the writeback, and is often the main initiator of 
large requests.  The VM thresholds also serve to keep request size 
manageable.  See e.g.
http://marc.theaimsgroup.com/?l=linux-kernel&m=108043321326801&w=2

IO scheduler:  the place to make the decision about whether the request 
latency is meeting expectations, etc.  It should be straightforward to 
use a windowing algorithm to slowly increase the request size until (a) 
latency limits are reached, (b) hardware limits are reached, or (c) VM 
thresholds are reached.

Ultimately there must be some -global- management of I/O, otherwise VM 
cannot survive, e.g. 128k requests on 1000 disks :)


> the max size of a request is something that should be set internally to
> the blkdev layer (at a lower level than the I/O scheduler or the VM
> layer).

Yes, I agree.

My point is there are two maximums:

1) the hardware limit
2) the limit that "makes sense", e.g. 512k or 1M for most

The driver should only care about #1, and should be "told" #2.

A very, very, very minimal implementation could be this:

--- 1.138/include/linux/blkdev.h        Fri Mar 12 04:33:07 2004
+++ edited/include/linux/blkdev.h       Sun Mar 28 22:44:15 2004
@@ -607,6 +607,24 @@

  extern void drive_stat_acct(struct request *, int, int);

+#define BLK_DISK_MAX_SECTORS   2048
+#define BLK_FLOPPY_MAX_SECTORS 64


Hardcoding such a maximum in the driver is inflexible and IMO incorrect.


> If one day things will change and the harddisk will require 32M large
> DMA transactions to keep up with the speed of the disk, the thing should
> be still solved during disk discovery inside the blkdev layer. The

32M is probably too large, but 1M is probably too small for:
a RAID array with 33 disks, that presents itself as a single SATA disk.
solid-state storage:  battery-backed RAM.

These things like bigger requests, and were designed to solve a lot of 
the latency problems in hardware.


> "automagic" suggestions discussed by Jamie and Jens should be just
> benchmarks internal to the blkdev layer, trying to read contigously
> first with 1M then 2M then 4M etc..  until the speed difference goes
> below 1% or whatever similar "autotune" algorithm.

Yes, agreed.

My main goal is to -not- worry about this in the low-level driver.   If 
you and Jens think 1M requests are maximum for disks, then put that in 
the _blkdev_ layer not my driver :)

Long term, I would like to see something like

--- 1.138/include/linux/blkdev.h        Fri Mar 12 04:33:07 2004
+++ edited/include/linux/blkdev.h       Sun Mar 28 23:01:42 2004
@@ -337,7 +337,8 @@
          */
         unsigned long           nr_requests;    /* Max # of requests */

-       unsigned short          max_sectors;
+       unsigned short          max_sectors;    /* blk layer-chosen */
+       unsigned short          max_hw_sectors; /* hardware limit */
         unsigned short          max_phys_segments;
         unsigned short          max_hw_segments;