IDE TCQ #2

echo "using_tcq:0" > /proc/ide/hdX/setting

will disable TCQ and revert to DMA,

echo "using_tcq:32" > /proc/ide/hdX/setting

will set queue depth to 32, any value in between the two are of course also allowed. The driver will print enable/disable info to the kernel log.

diff -urN -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.5.8-pre2/drivers/block/ll_rw_blk.c linux/drivers/block/ll_rw_blk.c

drivers/block/ll_rw_blk.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/block/ll_rw_blk.c +2 -2

@@ -857,10 +857,10 @@
 	spin_lock_prefetch(q->queue_lock);
 
 	generic_unplug_device(q);
-	add_wait_queue(&rl->wait, &wait);
+	add_wait_queue_exclusive(&rl->wait, &wait);
 	do {
 		set_current_state(TASK_UNINTERRUPTIBLE);
-		if (rl->count < batch_requests)
+		if (!rl->count)
 			schedule();
 		spin_lock_irq(q->queue_lock);
 		rq = get_request(q, rw);

@@ -1683,9 +1683,11 @@
 	 * Free request slots per queue.

diff -urN -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-disk.c linux/drivers/ide/ide-disk.c

drivers/ide/ide-disk.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-disk.c +2 -1

@@ -26,9 +26,10 @@
  * Version 1.11		Highmem I/O support, Jens Axboe <[email protected]>
  * Version 1.12		added 48-bit lba
  * Version 1.13		adding taskfile io access method
+ * Version 1.14		Added tcq support, Jens Axboe <[email protected]>
  */
 
-#define IDEDISK_VERSION	"1.13"
+#define IDEDISK_VERSION	"1.14"
 
 #include <linux/config.h>
 #include <linux/module.h>

drivers/ide/ide-disk.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-disk.c +9 -0

+int ide_start_dma(struct ata_channel *hwif, ide_drive_t *drive, ide_dma_action_t func)
+{
+	unsigned int reading = 0, count;
+	unsigned long dma_base = hwif->dma_base;
+	ata_request_t *ar = IDE_CUR_AR(drive);
+
+	if (rq_data_dir(ar->ar_rq) == READ)
+		reading = 1 << 3;
+

diff -urN -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-features.c linux/drivers/ide/ide-features.c

Quoted 4 lines

echo "using_tcq:0" > /proc/ide/hdX/setting

will disable TCQ and revert to DMA,

Quoted 5 lines

echo "using_tcq:32" > /proc/ide/hdX/setting

will set queue depth to 32, any value in between the two are of course also allowed. The driver will print enable/disable info to the kernel log.

Well this belongs to an ioctl or sysctl... However most of the /proc/ide stuff if not all will go to the sysctl quite soon.

drivers/ide/ide-disk.c line 113

Quoted 7 lines

#include <linux/config.h>
#include <linux/module.h>

drivers/ide/ide-disk.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-disk.c +1 -0

@@ -109,53 +110,64 @@
 static u8 get_command(ide_drive_t *drive, int cmd)
 {
 	int lba48bit = (drive->id->cfs_enable_2 & 0x0400) ? 1 : 0;
+	int command = WIN_NOP;

The "incremental" usage of the command variable I think is an overoptimization.

u8 is the proper type for it anyway. I have unwound the code below once to make it more obvious and you would be surprised what GCC does with it :-).

drivers/ide/ide-disk.c line 162

Quoted 3 lines

drivers/ide/ide-disk.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-disk.c +2 -0

+
+	return command;
 }

See you are returning an int entity as u8!

Review line 118

Quoted 2 lines

(quoted diff) +2 -0

+	ide_task_t			*args = &ar->ar_task;
+	struct request			*rq = ar->ar_rq;

Hints that ide_task_t and ata_request_t and struct request usage belong together. This could alleviate the ugly usage of the rq->special field if rq is struct request - which would be a GoodThin(TM).

drivers/ide/ide-disk.c line 223

Quoted 10 lines

drivers/ide/ide-disk.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-disk.c +4 -4

 n ata_taskfile(drive,
-			&args.taskfile,
-			&args.hobfile,
-			args.handler,
-			args.prehandler,
+			&args->taskfile,
+			&args->hobfile,
+			args->handler,
+			args->prehandler,
 			rq);

Due to the other cleanups which happened already it was now possible for me to collapse the arguments of the ata_taskfile function to use only one parameter - names ide_task_t. This will immediately allow to consolidate all data specific to a command into one queued structure, which will be named struct ata_request then. I will post the corresponding patch just immediately, since the integration of the tcq stuff will be quite involved with it.

Quoted 4 lines

(quoted diff) +2 -0

+/*
+ * FIXME: taskfiles should be a map of pages, not a long virt address... 

/jens
+ */

I fully agree with you - possible it will be helpfull to just pee at the SCSI code to see how it should be done...

drivers/ide/ide-dma.c line 307

Quoted 4 lines

drivers/ide/ide-dma.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-dma.c +4 -0

+	if (rq->flags & REQ_DRIVE_TASKFILE)
+		ar->ar_sg_nents = ide_raw_build_sglist(hwif, rq);
+	else 
+		ar->ar_sg_nents = ide_build_sglist(hwif, rq);

Most certainly the switch should be pushed down to one sinde build_sglist function - we are passing the rq anyway to it.

drivers/ide/ide-dma.c line 381

Quoted 7 lines

drivers/ide/ide-dma.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-dma.c +1 -2

@@ -372,10 +378,9 @@
 void ide_destroy_dmatable (ide_drive_t *drive)
 {
 	struct pci_dev *dev = drive->channel->pci_dev;
-	struct scatterlist *sg = drive->channel->sg_table;
-	int nents = drive->channel->sg_nents;
+	ata_request_t *ar = IDE_CUR_AR(drive);

This looks a bit ugly and should be analogous to ata_ar_get() possible ata_ar_current() will fit.

Quoted 19 lines

drivers/ide/ide-dma.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-dma.c +18 -0

+int ide_start_dma(struct ata_channel *hwif, ide_drive_t *drive, 
 ide_dma_action_t func)
+{
+	unsigned int reading = 0, count;
+	unsigned long dma_base = hwif->dma_base;
+	ata_request_t *ar = IDE_CUR_AR(drive);
+
+	if (rq_data_dir(ar->ar_rq) == READ)
+		reading = 1 << 3;
+
+	if (hwif->rwproc)
+		hwif->rwproc(drive, func);
+
+	if (!(count = ide_build_dmatable(drive, ar->ar_rq, func)))
+		return 1;	/* try PIO instead of DMA */
+
+	ar->ar_flags |= ATA_AR_SETUP;
+	outl(ar->ar_dmatable, dma_base + 4);	/* PRD table */
+	outb(reading, dma_base);		/* specify r/w */

Quoted 2 lines

(quoted diff) +1 -0

+	outb(inb(dma_base + 2) | 6, dma_base+2);/* clear INTR & ERROR flags 

*/

Hmmm there is one architecture which will have possbile problems with this namely cris. But right now I'm quite convinced that they should fix something
there and the OUT_ IDE code specific compatibility macros should be removed altogether.

Quoted 11 lines

diff -urN -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-features.c linux/drivers/ide/ide-features.c
--- /opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-features.c Tue Apr 9
11:41:13 2002
+++ linux/drivers/ide/ide-features.c Thu Apr 4 08:14:18 2002

drivers/ide/ide-features.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-features.c +0 -1

@@ -75,10 +75,7 @@
 char *ide_dmafunc_verbose (ide_dma_action_t dmafunc)
 {
 	switch (dmafunc) {
-		case ide_dma_read:		return("ide_dma_read");

drivers/ide/ide-features.c line 81

Quoted 6 lines

drivers/ide/ide-features.c renamed from opt/kernel/linux-2.5.8-pre2/drivers/ide/ide-features.c +0 -2

-		case ide_dma_write:		return("ide_dma_write");
 		case ide_dma_begin:		return("ide_dma_begin");
-		case ide_dma_end:		return("ide_dma_end:");
 		case ide_dma_check:		return("ide_dma_check");
 		case ide_dma_on:		return("ide_dma_on");
 		case ide_dma_off:		return("ide_dma_off");

This abortion will be killed by using __FUNCTION__ where apriopriate in debugging code.

OK I will just "eat" your code this evening...

Hi,

Version 2 is ready. Changes since last time:

Quoted 3 lines · Jens Axboe

Hi,

Version 2 is ready. Changes since last time:

Hi,

OK I have managed to merge this with the 2.5.8-pre2 + ide-29b at home. However since we have now apparently already a -pre3 I will have to at least redo the patches against it. If this takes more then the time needed for a cup of coffe I will have unfortuntely to do it today afternoon.

Quoted 6 lines · Martin Dalecki

Quoted 3 lines · Jens Axboe

Hi,

Version 2 is ready. Changes since last time:

Quoted 7 lines

Hi,

OK I have managed to merge this with the 2.5.8-pre2 + ide-29b at home. However since we have now apparently already a -pre3 I will have to at least redo the patches against it. If this takes more then the time needed for a cup of coffe I will have unfortuntely to do it today afternoon.

I'm already running 2.5.8-pre3 (which appears to include ide-29b, right?) + ide-tcq here, so you should probably not waste any effort on that :-)

I'll post an updated patch later today.

Quoted 17 lines · Jens Axboe

Quoted 13 lines · Martin Dalecki

Quoted 3 lines · Jens Axboe

Hi,

Version 2 is ready. Changes since last time:

Hi,

OK I have managed to merge this with the 2.5.8-pre2 + ide-29b at home. However since we have now apparently already a -pre3 I will have to at least redo the patches against it. If this takes more then the time needed for a cup of coffe I will have unfortuntely to do it today afternoon.

Quoted 3 lines

I'm already running 2.5.8-pre3 (which appears to include ide-29b, right?) + ide-tcq here, so you should probably not waste any effort on that :-)

Well than I will have to basically redo all the stuff I did in between... like for example nuking the number of parameters to ata_taskfile()... :-(.

Quoted 7 lines

Quoted 3 lines

I'm already running 2.5.8-pre3 (which appears to include ide-29b, right?) + ide-tcq here, so you should probably not waste any effort on that :-)

Well than I will have to basically redo all the stuff I did in between... like for example nuking the number of parameters to ata_taskfile()... :-(.

Ok, go ahead with the merge then and keep your changes. I'll just diff #2 -> current and apply that to whatever shows up in the next kernel.