take 48-bit lba a bit further
PATCH 1/1 take 48-bit lba a bit further
Hi,
Thanks for taking the previous bit Alan, here's an incremental update to 2.5.66-ac2. Just cleans up the 'when to use 48-bit lba' logic a bit per Andries suggestion, and also expands the request size for 48-bit lba capable drives to 512KiB.
Works perfectly in testing here, ext2/3 generates nice big 512KiB requests and the drive flies.
drivers/ide/ide-disk.c renamed from opt/kernel/linux-2.5.66-ac2/drivers/ide/ide-disk.c +8 -5
@@ -373,7 +373,7 @@ nsectors.all = (u16 ) rq->nr_sectors; -if (drive->addressing ==1 && block + rq->nr_sectors >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48 =1 ;if (driver_blocked)
@@ -583,7 +583,7 @@ {int lba48bit =0 ; -if (drive->addressing ==1 && block >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48bit =1 ;BUG_ON (drive->blocked);
@@ -611,7 +611,7 @@return ide_started; } -if (lba48bit && block >0xfffffff ) +if (lba48bit)return lba_48_rw_disk (drive, rq, (unsigned long long ) block);if (drive->select.b.lba)/* 28-bit LBA */ return lba_28_rw_disk (drive, rq, (unsigned long ) block);
@@ -625,7 +625,7 @@int cmd =rq_data_dir (rq);int lba48bit =0 ; -if (drive->addressing ==1 && rq->sector >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48bit =1 ;if ((cmd ==READ ) && drive->using_tcq)
@@ -1504,7 +1504,7 @@static int set_lba_addressing (ide_drive_t *drive,int arg) { -return (probe_lba_addressing (drive, arg)); +return probe_lba_addressing (drive, arg); }static void idedisk_add_settings (ide_drive_t *drive)
@@ -1591,6 +1591,9 @@ (void )probe_lba_addressing (drive,1 ); +if (drive->addressing) +blk_queue_max_sectors (&drive->queue,1024 ); +/* Extract geometry if we did not already have one for the drive */ if (!drive->cyl || !drive->head || !drive->sect) { drive->cyl = drive->bios_cyl = id->cyls;
drivers/ide/ide-dma.c renamed from opt/kernel/linux-2.5.66-ac2/drivers/ide/ide-dma.c +2 -2
@@ -663,7 +663,7 @@if (drive->media != ide_disk)return 0 ; -if (drive->addressing ==1 && rq->sector >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48 =1 ; command = (lba48) ?WIN_READDMA_EXT :WIN_READDMA ;
@@ -698,7 +698,7 @@if (drive->media != ide_disk)return 0 ; -if (drive->addressing ==1 && rq->sector >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48 =1 ; command = (lba48) ?WIN_WRITEDMA_EXT :WIN_WRITEDMA ;
include/linux/ide.h renamed from opt/kernel/linux-2.5.66-ac2/include/linux/ide.h +6 -0
@@ -866,6 +866,12 @@bio_kunmap_irq (buffer, flags); } +/* +* must be addressed with 48-bit lba +*/ +#define rq_lba48 (rq) \ + (((rq)->sector + (rq)->nr_sectors) > 0xfffffff || rq->nr_sectors > 256) +#define IDE_CHIPSET_PCI_MASK \ ((1<<ide_pci)|(1<<ide_cmd646)|(1<<ide_ali14xx))#define IDE_CHIPSET_IS_PCI (c) ((IDE_CHIPSET_PCI_MASK >> (c)) & 1)
Signature · 1 line
Jens Axboe
Quoted 7 lines
Thanks for taking the previous bit Alan, here's an incremental update to 2.5.66-ac2. Just cleans up the 'when to use 48-bit lba' logic a bit per Andries suggestion, and also expands the request size for 48-bit lba capable drives to 512KiB.
Works perfectly in testing here, ext2/3 generates nice big 512KiB requests and the drive flies.
Then, don't we want to be using 48-bit lba all the time on compatible devices instead of falling back to 28-bit when possible to save a small amount of instruction overhead? (Or is that what we're doing already? I haven't really had the time to follow this thread).
John.
Quoted 5 lines · John Bradford
Quoted 5 lines
Thanks for taking the previous bit Alan, here's an incremental update to 2.5.66-ac2. Just cleans up the 'when to use 48-bit lba' logic a bit per Andries suggestion, and also expands the request size for 48-bit lba capable drives to 512KiB.
Quoted 7 lines
Quoted 2 lines
Works perfectly in testing here, ext2/3 generates nice big 512KiB requests and the drive flies.
Then, don't we want to be using 48-bit lba all the time on compatible devices instead of falling back to 28-bit when possible to save a small amount of instruction overhead? (Or is that what we're doing already? I haven't really had the time to follow this thread).
The logic in the patch is to enable large requests _if_ the drive can do 48-bit lba. However, we will only use 48-bit lba commands if the request is either beyond 2^28 sectors _or_ bigger than 256 sectors since neither of these can be addressed with 28-bit lba.
See rq_lba48 in the patch, it explains it.
Signature · 1 line
Jens Axboe
Quoted 4 lines · John Bradford
Then, don't we want to be using 48-bit lba all the time on compatible devices instead of falling back to 28-bit when possible to save a small amount of instruction overhead? (Or is that what we're doing already? I haven't really had the time to follow this thread).
The overhead of the double load of the command registers is microseconds so it is actually quite a lot, especially since IDE lacks TCQ so neither end of the link is doing *anything* useful. SCSI has similar problems on older SCSI with command sending being slow, but the drive is at least doing other commands during this.
Quoted 10 lines
Quoted 4 lines
Then, don't we want to be using 48-bit lba all the time on compatible devices instead of falling back to 28-bit when possible to save a small amount of instruction overhead? (Or is that what we're doing already? I haven't really had the time to follow this thread).
The overhead of the double load of the command registers is microseconds so it is actually quite a lot, especially since IDE lacks TCQ so neither end of the link is doing *anything* useful. SCSI has similar problems on older SCSI with command sending being slow, but the drive is at least doing other commands during this.
So, say you have a choice of either a 256Kb request to a low block number, which can use the faster 28-bit mode, or a 512Kb request to the same low block number, which can only be made using 48-bit LBA, which is the best to use?
I originally thought that we might only be honouring 512Kb requests for blocks over the 28-bit limit, which Jens corrected me on, but maybe we *should* only do 512Kb requests on high block number, where we have to use 48-bit anyway.
John.
Quoted 4 lines · John Bradford
I originally thought that we might only be honouring 512Kb requests for blocks over the 28-bit limit, which Jens corrected me on, but maybe we *should* only do 512Kb requests on high block number, where we have to use 48-bit anyway.
That makes little sense in practice, and is not currently even doable within the block layer. You got the limits wrong, btw, it's 128kb max for 28-bit. A single 512KiB request will have a lower per-kb overhead with 48-bit lba than a single 128kb on 28-bit would.
Signature · 1 line
Jens Axboe
Quoted 10 lines · Jens Axboe
Quoted 4 lines · John Bradford
I originally thought that we might only be honouring 512Kb requests for blocks over the 28-bit limit, which Jens corrected me on, but maybe we *should* only do 512Kb requests on high block number, where we have to use 48-bit anyway.
That makes little sense in practice, and is not currently even doable within the block layer. You got the limits wrong, btw, it's 128kb max for 28-bit. A single 512KiB request will have a lower per-kb overhead with 48-bit lba than a single 128kb on 28-bit would.
I should mention that the 512KiB number is one I chose, as a good large request size. You could as high as 32MiB.
Signature · 1 line
Jens Axboe
Quoted 2 lines
A single 512KiB request will have a lower per-kb overhead with 48-bit lba than a single 128kb on 28-bit would.
Ah, OK, that's what I wasn't sure about.
John.
Quoted 10 lines · Jens Axboe
Hi,
Thanks for taking the previous bit Alan, here's an incremental update to 2.5.66-ac2. Just cleans up the 'when to use 48-bit lba' logic a bit per Andries suggestion, and also expands the request size for 48-bit lba capable drives to 512KiB.
Works perfectly in testing here, ext2/3 generates nice big 512KiB requests and the drive flies.
drivers/ide/ide-disk.c line 1596
Quoted 5 lines
drivers/ide/ide-disk.c renamed from opt/kernel/linux-2.5.66-ac2/drivers/ide/ide-disk.c +3
-0(void )probe_lba_addressing (drive,1 ); +if (drive->addressing) +blk_queue_max_sectors (&drive->queue,1024 ); +
Should not you honor host's max queue length? siimage & pdc4030 sets max_sectors to 128 (resp. 16 resp. 127), while you overwrite it here unconditionally with 1024.
Thanks,
Petr Vandrovec [email protected]
Quoted 5 lines · Petr Vandrovec
Quoted 5 lines
Works perfectly in testing here, ext2/3 generates nice big 512KiB requests and the drive flies.
(void)
probe_lba_addressing(drive, 1);
Quoted 7 lines
drivers/ide/ide-disk.c line 1596
Quoted 3 lines
drivers/ide/ide-disk.c renamed from opt/kernel/linux-2.5.66-ac2/drivers/ide/ide-disk.c +3
-0+if (drive->addressing) +blk_queue_max_sectors (&drive->queue,1024 ); +Should not you honor host's max queue length? siimage & pdc4030 sets
max_sectorsto 128 (resp. 16 resp. 127), while you overwrite it here unconditionally with 1024.
For production code thats required. The actual change required is to clamp the default/hwif set queue limit to 256 if driver->addressing !=1
Quoted 14 lines · Alan Cox
Quoted 5 lines · Petr Vandrovec
Quoted 5 lines
Works perfectly in testing here, ext2/3 generates nice big 512KiB requests and the drive flies.
(void)
probe_lba_addressing(drive, 1);Quoted 7 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+ if (drive->addressing) + blk_queue_max_sectors(&drive->queue, 1024); +Should not you honor host's max queue length? siimage & pdc4030 sets
max_sectorsto 128 (resp. 16 resp. 127), while you overwrite it here unconditionally with 1024.
Quoted 2 lines
For production code thats required. The actual change required is to clamp the default/hwif set queue limit to 256 if
driver->addressing!=1
Yes I was aware of this short-cut. I want to see rqsize being generally set as a maximum, of the host adapter doesn't set its own upper limit. This patch adds that little piece (I think I caught both legacy and pci, please double check Alan), and also fixes pdc202xx_old to work with 48-bit again.
drivers/ide/ide-disk.c renamed from opt/kernel/linux-2.5.66-ac2/drivers/ide/ide-disk.c +14 -5
@@ -373,7 +373,7 @@ nsectors.all = (u16 ) rq->nr_sectors; -if (drive->addressing ==1 && block + rq->nr_sectors >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48 =1 ;if (driver_blocked)
@@ -583,7 +583,7 @@ {int lba48bit =0 ; -if (drive->addressing ==1 && block >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48bit =1 ;BUG_ON (drive->blocked);
@@ -611,7 +611,7 @@return ide_started; } -if (lba48bit && block >0xfffffff ) +if (lba48bit)return lba_48_rw_disk (drive, rq, (unsigned long long ) block);if (drive->select.b.lba)/* 28-bit LBA */ return lba_28_rw_disk (drive, rq, (unsigned long ) block);
@@ -625,7 +625,7 @@int cmd =rq_data_dir (rq);int lba48bit =0 ; -if (drive->addressing ==1 && rq->sector >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48bit =1 ;if ((cmd ==READ ) && drive->using_tcq)
@@ -1504,7 +1504,7 @@static int set_lba_addressing (ide_drive_t *drive,int arg) { -return (probe_lba_addressing (drive, arg)); +return probe_lba_addressing (drive, arg); }static void idedisk_add_settings (ide_drive_t *drive)
@@ -1591,6 +1591,15 @@ (void )probe_lba_addressing (drive,1 ); +if (drive->addressing) { +int max_sectors =1024 ; + +if (max_sectors >HWIF (drive)->rqsize) + max_sectors =HWIF (drive)->rqsize; + +blk_queue_max_sectors (&drive->queue, max_sectors); + } +/* Extract geometry if we did not already have one for the drive */ if (!drive->cyl || !drive->head || !drive->sect) { drive->cyl = drive->bios_cyl = id->cyls;
drivers/ide/ide-dma.c renamed from opt/kernel/linux-2.5.66-ac2/drivers/ide/ide-dma.c +2 -2
@@ -663,7 +663,7 @@if (drive->media != ide_disk)return 0 ; -if (drive->addressing ==1 && rq->sector >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48 =1 ; command = (lba48) ?WIN_READDMA_EXT :WIN_READDMA ;
@@ -698,7 +698,7 @@if (drive->media != ide_disk)return 0 ; -if (drive->addressing ==1 && rq->sector >0xfffffff ) +if (drive->addressing ==1 &&rq_lba48 (rq)) lba48 =1 ; command = (lba48) ?WIN_WRITEDMA_EXT :WIN_WRITEDMA ;
drivers/ide/ide.c renamed from opt/kernel/linux-2.5.66-ac2/drivers/ide/ide.c +3 -0
@@ -947,6 +947,9 @@#endif } +if (!hwif->rqsize) + hwif->rqsize =65535 ; +if (hwifp) *hwifp = hwif;
drivers/ide/pci/pdc202xx_old.c renamed from opt/kernel/linux-2.5.66-ac2/drivers/ide/pci/pdc202xx_old.c +6 -3
@@ -535,8 +535,9 @@static int pdc202xx_old_ide_dma_begin (ide_drive_t *drive) { -if (drive->addressing ==1 ) { -struct request *rq =HWGROUP (drive)->rq; +struct request *rq =HWGROUP (drive)->rq; + +if (drive->addressing ==1 &&rq_lba48 (rq)) {ide_hwif_t *hwif =HWIF (drive);// struct pci_dev *dev = hwif->pci_dev; // unsgned long high_16 = pci_resource_start(dev, 4);
@@ -557,7 +558,9 @@static int pdc202xx_old_ide_dma_end (ide_drive_t *drive) { -if (drive->addressing ==1 ) { +struct request *rq =HWGROUP (drive)->rq; + +if (drive->addressing ==1 &&rq_lba48 (rq)) {ide_hwif_t *hwif =HWIF (drive);// unsigned long high_16 = pci_resource_start(hwif->pci_dev, 4); unsigned long high_16 = hwif->dma_master;
drivers/ide/setup-pci.c renamed from opt/kernel/linux-2.5.66-ac2/drivers/ide/setup-pci.c +3 -1
@@ -632,7 +632,6 @@ index->b.low = hwif->index; } -if (d->init_iops) d->init_iops (hwif);
@@ -658,6 +657,9 @@*/ d->init_hwif (hwif); +if (!hwif->rqsize) + hwif->rqsize =65535 ; +/* * This is in the wrong place. The driver may * do set up based on the autotune value and this
include/linux/ide.h renamed from opt/kernel/linux-2.5.66-ac2/include/linux/ide.h +6 -0
@@ -866,6 +866,12 @@bio_kunmap_irq (buffer, flags); } +/* +* must be addressed with 48-bit lba +*/ +#define rq_lba48 (rq) \ + (((rq)->sector + (rq)->nr_sectors) > 0xfffffff || rq->nr_sectors > 256) +#define IDE_CHIPSET_PCI_MASK \ ((1<<ide_pci)|(1<<ide_cmd646)|(1<<ide_ali14xx))#define IDE_CHIPSET_IS_PCI (c) ((IDE_CHIPSET_PCI_MASK >> (c)) & 1)
Signature · 1 line
Jens Axboe