linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* PS/2 Esdi patch #8
@ 2001-05-23 22:54 Hal Duston
  2001-05-24 20:46 ` Paul Gortmaker
  0 siblings, 1 reply; 8+ messages in thread
From: Hal Duston @ 2001-05-23 22:54 UTC (permalink / raw)
  To: linux-kernel; +Cc: alan

[-- Attachment #1: Type: TEXT/PLAIN, Size: 364 bytes --]

Get soft reset at initialization working again.

Remove reset_time code.  (Re-add timeouts some time later.)
Eliminate remaining sleep_on() calls.
Encapsulate access to esdi attention register.
Correct some formatting in config display.
Correct soft reset logic.

Also...
http://www.sound.net/~hald/projects/ps2esdi/ps2esdi-2.4.4-patch4

Hal Duston
hald@sound.net

[-- Attachment #2: Type: TEXT/PLAIN, Size: 10456 bytes --]

Get soft reset at initialization working again.

Remove reset_time code.  (Re-add timeouts some time later.)
Eliminate remaining sleep_on() calls.
Encapsulate access to esdi attention register.
Correct some formatting in config display.
Correct soft reset logic.

--- linux-2.4.5-pre4/drivers/block/ps2esdi.c.3	Mon May 21 00:07:38 2001
+++ linux-2.4.5-pre4/drivers/block/ps2esdi.c.4	Mon May 21 00:16:49 2001
@@ -107,6 +107,8 @@
 
 static void ps2esdi_reset_timer(unsigned long unused);
 
+static int ps2esdi_attention(u_int device, u_int request);
+
 static u_int dma_arb_level;		/* DMA arbitration level */
 
 static DECLARE_WAIT_QUEUE_HEAD(ps2esdi_int);
@@ -460,19 +462,8 @@
 	current_int_handler = ps2esdi_initial_reset_int_handler;
 	reset_ctrl();
 	reset_status = 0;
-	reset_start = jiffies;
-	while (!reset_status) {
-		init_timer(&esdi_timer);
-		esdi_timer.expires = jiffies + HZ;
-		esdi_timer.data = 0;
-		add_timer(&esdi_timer);
-		sleep_on(&ps2esdi_int);
-	}
-	reset_end = jiffies;
+	wait_event(ps2esdi_int, reset_status);
 	LITE_OFF;
-	printk("%s: reset interrupt after %d jiffies,  %u.%02u secs\n",
-	       DEVICE_NAME, reset_end - reset_start, (reset_end - reset_start) / HZ,
-	       (reset_end - reset_start) % HZ);
 
 
 	/* Integrated ESDI Disk and Controller has only one drive! */
@@ -522,8 +513,7 @@
 		cmd_blk[1] = 0;
 		no_int_yet = TRUE;
 		ps2esdi_out_cmd_blk(cmd_blk);
-		if (no_int_yet)
-			sleep_on(&ps2esdi_int);
+		wait_event(ps2esdi_int, !no_int_yet);
 	}
 	return;
 }
@@ -597,32 +587,20 @@
 {
 
 	u_long expire;
-	u_short status;
-
-	/* enable interrupts on the controller */
-	status = inb(ESDI_INTRPT);
-	outb((status & 0xe0) | ATT_EOI, ESDI_ATTN);	/* to be sure we don't have
-							   any interrupt pending... */
-	outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
 
-	/* read the ESDI status port - if the controller is not busy,
-	   simply do a soft reset (fast) - otherwise we'll have to do a
-	   hard (slow) reset.  */
-	if (!(inb(ESDI_STATUS) & STATUS_BUSY)) {
+	if(!ps2esdi_attention(0x07, 0x04)) {
 		/*BA */ printk("%s: soft reset...\n", DEVICE_NAME);
-		outb(CTRL_SOFT_RESET, ESDI_ATTN);
 	}
-	/* soft reset */ 
+	/* soft reset */
 	else {
 		/*BA */
 		printk("%s: hard reset...\n", DEVICE_NAME);
 		outb(CTRL_HARD_RESET, ESDI_CONTROL);
-		expire = jiffies + 2*HZ;
-		while (time_before(jiffies, expire));
-		outb(1, ESDI_CONTROL);
+		expire = jiffies + 2 * HZ;
+		while(time_before(jiffies, expire))
+		{}
+		outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
 	}			/* hard reset */
-
-
 }				/* reset the controller */
 
 /* called by the strategy routine to handle read and write requests */
@@ -695,13 +673,10 @@
 	printk("%s: i(1)=%d\n", DEVICE_NAME, i);
 #endif
 
-	/* if device is still busy - then just time out */
-	if (inb(ESDI_STATUS) & STATUS_BUSY) {
-		printk("%s: ps2esdi_out_cmd timed out (1)\n", DEVICE_NAME);
+	if(ps2esdi_attention((cmd_blk[0] & 0xe0) >> 5, 0x01)) {
+		printk("%s: ps2esdi_out_cmd timed out (%x)\n", DEVICE_NAME, (cmd_blk[0] & 0xe0) >> 5);
 		return ERROR;
-	}			/* timeout ??? */
-	/* Set up the attention register in the controller */
-	outb(((*cmd_blk) & 0xE0) | 1, ESDI_ATTN);
+	}
 
 #if 0
 	printk("%s: sending %d words to controller\n", DEVICE_NAME, (((*cmd_blk) >> 14) + 1) << 1);
@@ -774,25 +749,25 @@
 static void ps2esdi_initial_reset_int_handler(u_int int_ret_code)
 {
 
+	reset_status = int_ret_code;
 	switch (int_ret_code & 0xf) {
 	case INT_RESET:
 		/*BA */
 		printk("%s: initial reset completed.\n", DEVICE_NAME);
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		wake_up(&ps2esdi_int);
 		break;
 	case INT_ATTN_ERROR:
-		printk("%s: Attention error. interrupt status : %02X\n", DEVICE_NAME,
+		printk("%s: Attention error during reset. interrupt status : %02X\n", DEVICE_NAME,
 		       int_ret_code);
 		printk("%s: status: %02x\n", DEVICE_NAME, inb(ESDI_STATUS));
 		break;
 	default:
 		printk("%s: initial reset handler received interrupt: %02X\n",
 		       DEVICE_NAME, int_ret_code);
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		break;
 	}
-	outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
 }
 
 
@@ -821,10 +796,10 @@
 				printk("%s: Device Configuration Status for drive %u\n",
 				       DEVICE_NAME, drive_num);
 
-				printk("%s: Spares/cyls: %u", DEVICE_NAME, reply[0] >> 8);
+				printk("%s: Spares/cyls: %u\n", DEVICE_NAME, reply[0] >> 8);
 
 				printk
-				    ("Config bits: %s%s%s%s%s\n",
+				    ("%s: Config bits: %s%s%s%s%s\n", DEVICE_NAME,
 				     (reply[0] & CONFIG_IS) ? "Invalid Secondary, " : "",
 				     ((reply[0] & CONFIG_ZD) && !(reply[0] & CONFIG_IS))
 				 ? "Zero Defect, " : "Defects Present, ",
@@ -877,11 +852,11 @@
 			printk("%s: command %02X unknown by geometry handler\n",
 			       DEVICE_NAME, status & 0x1f);
 
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		break;
 
 	case INT_ATTN_ERROR:
-		printk("%s: Attention error. interrupt status : %02X\n", DEVICE_NAME,
+		printk("%s: Attention error during geometry. interrupt status : %02X\n", DEVICE_NAME,
 		       int_ret_code);
 		printk("%s: Device not available\n", DEVICE_NAME);
 		--ps2esdi_drives;
@@ -896,19 +871,18 @@
 	case INT_CMD_BLK_ERR:
 		/*BA */ printk("%s: Whaa. Error occurred...\n", DEVICE_NAME);
 		dump_cmd_complete_status(int_ret_code);
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		--ps2esdi_drives;
 		break;
 	default:
 		printk("%s: Unknown interrupt reason: %02X\n",
 		       DEVICE_NAME, int_ret_code & 0xf);
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		break;
 	}
 
 	wake_up(&ps2esdi_int);
 	no_int_yet = FALSE;
-	outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
 
 }
 
@@ -930,7 +904,7 @@
 		break;
 
 	case INT_ATTN_ERROR:
-		printk("%s: Attention error. interrupt status : %02X\n", DEVICE_NAME,
+		printk("%s: Attention error during normal. interrupt status : %02X\n", DEVICE_NAME,
 		       int_ret_code);
 		outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
 		ending = FAIL;
@@ -940,8 +914,7 @@
 		for (i = ESDI_TIMEOUT; i & !(inb(ESDI_STATUS) & STATUS_STAT_AVAIL); i--);
 		if (!(inb(ESDI_STATUS) & STATUS_STAT_AVAIL)) {
 			printk("%s: timeout reading status word\n", DEVICE_NAME);
-			outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
-			outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+			ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 			if ((++CURRENT->errors) >= MAX_RETRIES)
 				ending = FAIL;
 			else
@@ -953,8 +926,7 @@
 		case (CMD_READ & 0xff):
 		case (CMD_WRITE & 0xff):
 			LITE_OFF;
-			outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
-			outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+			ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 #if 0
 			printk("ps2esdi: cmd_complete b_wait: %p\n", &CURRENT->bh->b_wait);
 #endif
@@ -963,8 +935,7 @@
 		default:
 			printk("%s: interrupt for unknown command %02X\n",
 			       DEVICE_NAME, status & 0x1f);
-			outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
-			outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+			ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 			ending = -1;
 			break;
 		}
@@ -974,8 +945,7 @@
 	case INT_CMD_ECC_RETRY:
 		LITE_OFF;
 		dump_cmd_complete_status(int_ret_code);
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
-		outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		ending = SUCCES;
 		break;
 	case INT_CMD_WARNING:
@@ -984,8 +954,7 @@
 	case INT_DMA_ERR:
 		LITE_OFF;
 		dump_cmd_complete_status(int_ret_code);
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
-		outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		if ((++CURRENT->errors) >= MAX_RETRIES)
 			ending = FAIL;
 		else
@@ -994,31 +963,27 @@
 
 	case INT_CMD_BLK_ERR:
 		dump_cmd_complete_status(int_ret_code);
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
-		outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		ending = FAIL;
 		break;
 
 	case INT_CMD_FORMAT:
 		printk("%s: huh ? Who issued this format command ?\n"
 		       ,DEVICE_NAME);
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
-		outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		ending = -1;
 		break;
 
 	case INT_RESET:
 		/* BA printk("%s: reset completed.\n", DEVICE_NAME) */ ;
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
-		outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		ending = -1;
 		break;
 
 	default:
 		printk("%s: Unknown interrupt reason: %02X\n",
 		       DEVICE_NAME, int_ret_code & 0xf);
-		outb((int_ret_code & 0xe0) | ATT_EOI, ESDI_ATTN);
-		outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+		ps2esdi_attention((int_ret_code & 0xe0) >> 5, ATT_EOI);
 		ending = -1;
 		break;
 	}
@@ -1125,8 +1090,7 @@
 	int dev = DEVICE_NR(inode->i_rdev);
 
 	if (dev < ps2esdi_drives) {
-		while (!ps2esdi_valid[dev])
-			sleep_on(&ps2esdi_wait_open);
+		wait_event(ps2esdi_wait_open, ps2esdi_valid[dev]);
 
 		access_count[dev]++;
 
@@ -1236,11 +1200,34 @@
 
 	status = inb(ESDI_INTRPT);
 	if ((status & 0xf) == INT_RESET) {
-		outb((status & 0xe0) | ATT_EOI, ESDI_ATTN);
-		outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
-		reset_status = 1;
+		ps2esdi_attention((status & 0xe0) >> 5, ATT_EOI);
+		reset_status = -1;
 	}
 	wake_up(&ps2esdi_int);
 }
 
+static int ps2esdi_attention(u_int device, u_int request)
+{
+	int busy;
+	u_char status;
+
+	outb(CTRL_DISABLE_INTR, ESDI_CONTROL);
+	if(request == ATT_EOI || !(status = inb(ESDI_STATUS) & 0x50)) {
+		outb((device << 5) | request, ESDI_ATTN);
+		busy = 0;
+	}
+	else {
+		if(status & 0x10) {
+			printk("%s: Attention port in use (0x%x)\n",
+				DEVICE_NAME, status);
+		}
+		if(status & 0x40) {
+			printk("%s: Interrupt pending (0x%x)\n",
+				DEVICE_NAME, status);
+		}
+ 		busy = 1;
+	}
+	outb(CTRL_ENABLE_INTR, ESDI_CONTROL);
+	return busy;
+}
 #endif

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PS/2 Esdi patch #8
  2001-05-23 22:54 PS/2 Esdi patch #8 Hal Duston
@ 2001-05-24 20:46 ` Paul Gortmaker
  2001-05-25 14:46   ` Jens Axboe
  0 siblings, 1 reply; 8+ messages in thread
From: Paul Gortmaker @ 2001-05-24 20:46 UTC (permalink / raw)
  To: Hal Duston; +Cc: linux-kernel, rasmus

Hal Duston wrote:

> http://www.sound.net/~hald/projects/ps2esdi/ps2esdi-2.4.4-patch4
> 
> Hal Duston
> hald@sound.net

You PS/2 ESDI guys might want to set the max sectors for your
driver - old default used to be 128, currently 255 (which maybe
hardware can handle ok?) - the xd and hd drivers were broken until
a similar fix was added to them.

Probably makes sense for driver to set it regardless, seeing 
as default (MAX_SECTORS) has changed several times over last
few months.  At least then it will be under driver control
and not at the mercy of some global value.

Paul.

--- drivers/block/ps2esdi.c~	Sun Apr 29 04:42:35 2001
+++ drivers/block/ps2esdi.c	Thu May 24 16:33:46 2001
@@ -117,6 +117,7 @@
 static char ps2esdi_valid[MAX_HD];
 static int ps2esdi_sizes[MAX_HD << 6];
 static int ps2esdi_blocksizes[MAX_HD << 6];
+static int ps2esdi_maxsect[MAX_HD << 6];
 static int ps2esdi_drives;
 static struct hd_struct ps2esdi[MAX_HD << 6];
 static u_short io_base;
@@ -414,8 +415,11 @@
 
 	ps2esdi_gendisk.nr_real = ps2esdi_drives;
 
-	for (i = 0; i < (MAX_HD << 6); i++)
+	/* 128 was old default, maybe maxsect=255 is ok too? - Paul G. */
+	for (i = 0; i < (MAX_HD << 6); i++) {
+		ps2esdi_maxsect[i] = 128;
 		ps2esdi_blocksizes[i] = 1024;
+	}
 
 	request_dma(dma_arb_level, "ed");
 	request_region(io_base, 4, "ed");




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PS/2 Esdi patch #8
  2001-05-24 20:46 ` Paul Gortmaker
@ 2001-05-25 14:46   ` Jens Axboe
  2001-05-26  9:53     ` Paul Gortmaker
  2001-05-26 14:49     ` A Duston
  0 siblings, 2 replies; 8+ messages in thread
From: Jens Axboe @ 2001-05-25 14:46 UTC (permalink / raw)
  To: Paul Gortmaker; +Cc: Hal Duston, linux-kernel, rasmus

On Thu, May 24 2001, Paul Gortmaker wrote:
> Hal Duston wrote:
> 
> > http://www.sound.net/~hald/projects/ps2esdi/ps2esdi-2.4.4-patch4
> > 
> > Hal Duston
> > hald@sound.net
> 
> You PS/2 ESDI guys might want to set the max sectors for your
> driver - old default used to be 128, currently 255 (which maybe
> hardware can handle ok?) - the xd and hd drivers were broken until
> a similar fix was added to them.
> 
> Probably makes sense for driver to set it regardless, seeing 
> as default (MAX_SECTORS) has changed several times over last
> few months.  At least then it will be under driver control
> and not at the mercy of some global value.

You might want to assign that max_sect array too, otherwise it's just
going to waste space :-)

Take a look at how ps2esdi handles requests -- always processing just
the first segment. Alas, it doesn't matter how big the request is.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PS/2 Esdi patch #8
  2001-05-25 14:46   ` Jens Axboe
@ 2001-05-26  9:53     ` Paul Gortmaker
  2001-05-26 14:49     ` A Duston
  1 sibling, 0 replies; 8+ messages in thread
From: Paul Gortmaker @ 2001-05-26  9:53 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Hal Duston, linux-kernel, rasmus

Jens Axboe wrote:
> 
> On Thu, May 24 2001, Paul Gortmaker wrote:
>
> > Probably makes sense for driver to set it regardless, seeing
> > as default (MAX_SECTORS) has changed several times over last
> > few months.  At least then it will be under driver control
> > and not at the mercy of some global value.
> 
> You might want to assign that max_sect array too, otherwise it's just
> going to waste space :-)

Heh, yes. :) 

> Take a look at how ps2esdi handles requests -- always processing just
> the first segment. Alas, it doesn't matter how big the request is.

Well here is the missing line, for completeness, in case somebody
someday gets bored and changes the above behaviour (it won't be me!)

Paul.

--- drivers/block/ps2esdi.c~	Thu May 24 16:33:46 2001
+++ drivers/block/ps2esdi.c	Sat May 26 04:47:45 2001
@@ -424,6 +424,7 @@
 	request_dma(dma_arb_level, "ed");
 	request_region(io_base, 4, "ed");
 	blksize_size[MAJOR_NR] = ps2esdi_blocksizes;
+	max_sectors[MAJOR_NR] = ps2esdi_maxsect;
 
 	for (i = 0; i < ps2esdi_drives; i++) {
 		register_disk(&ps2esdi_gendisk,MKDEV(MAJOR_NR,i<<6),1<<6,





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PS/2 Esdi patch #8
  2001-05-25 14:46   ` Jens Axboe
  2001-05-26  9:53     ` Paul Gortmaker
@ 2001-05-26 14:49     ` A Duston
  2001-05-26 14:58       ` Jens Axboe
  1 sibling, 1 reply; 8+ messages in thread
From: A Duston @ 2001-05-26 14:49 UTC (permalink / raw)
  To: Axboe, Jens; +Cc: Gortmaker, Paul, Andersen, Rasmus, linux-kernel

Jens Axboe wrote:

> On Thu, May 24 2001, Paul Gortmaker wrote:
> > Hal Duston wrote:
> >
> > > http://www.sound.net/~hald/projects/ps2esdi/ps2esdi-2.4.4-patch4
> > >
> > > Hal Duston
> > > hald@sound.net
> >
> > You PS/2 ESDI guys might want to set the max sectors for your
> > driver - old default used to be 128, currently 255 (which maybe
> > hardware can handle ok?) - the xd and hd drivers were broken until
> > a similar fix was added to them.
> >
> > Probably makes sense for driver to set it regardless, seeing
> > as default (MAX_SECTORS) has changed several times over last
> > few months.  At least then it will be under driver control
> > and not at the mercy of some global value.
>
> You might want to assign that max_sect array too, otherwise it's just
> going to waste space :-)
>
> Take a look at how ps2esdi handles requests -- always processing just
> the first segment. Alas, it doesn't matter how big the request is.

OK, obviously I am still missing something here from when I got the
driver booting again.  Presumably something with current_nr_sectors,
vs nr_sectors, maybe?  I thought it was odd that all the transfers were
exactly 2 blocks. I'll go ahead and take this one.  I will also go ahead
and check to see how much data the hardware can transfer at once
as well, but I expect it is quite a bit.  I am still working on getting a

group of folks to test patch #5 to see if it works as well.  Anyone
with appropriate hardware willing to test can contact me.  All I have
access to is my thinkpad 700 PS/2, and a 50Z that is a 286.

Thanks,
Hal Duston
hald@sound.net


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PS/2 Esdi patch #8
  2001-05-26 14:49     ` A Duston
@ 2001-05-26 14:58       ` Jens Axboe
  2001-05-26 15:33         ` A Duston
  0 siblings, 1 reply; 8+ messages in thread
From: Jens Axboe @ 2001-05-26 14:58 UTC (permalink / raw)
  To: A Duston; +Cc: Gortmaker, Paul, Andersen, Rasmus, linux-kernel

On Sat, May 26 2001, A Duston wrote:
> > On Thu, May 24 2001, Paul Gortmaker wrote:
> > > Hal Duston wrote:
> > >
> > > > http://www.sound.net/~hald/projects/ps2esdi/ps2esdi-2.4.4-patch4
> > > >
> > > > Hal Duston
> > > > hald@sound.net
> > >
> > > You PS/2 ESDI guys might want to set the max sectors for your
> > > driver - old default used to be 128, currently 255 (which maybe
> > > hardware can handle ok?) - the xd and hd drivers were broken until
> > > a similar fix was added to them.
> > >
> > > Probably makes sense for driver to set it regardless, seeing
> > > as default (MAX_SECTORS) has changed several times over last
> > > few months.  At least then it will be under driver control
> > > and not at the mercy of some global value.
> >
> > You might want to assign that max_sect array too, otherwise it's just
> > going to waste space :-)
> >
> > Take a look at how ps2esdi handles requests -- always processing just
> > the first segment. Alas, it doesn't matter how big the request is.
> 
> OK, obviously I am still missing something here from when I got the
> driver booting again.  Presumably something with current_nr_sectors,
> vs nr_sectors, maybe?  I thought it was odd that all the transfers were
> exactly 2 blocks. I'll go ahead and take this one.  I will also go ahead
> and check to see how much data the hardware can transfer at once
> as well, but I expect it is quite a bit.  I am still working on getting a

Consider the following request, with attached bh list:

	req -> bh1 -> bh2 -> bh3 -> bh4

Lets say this is a 4kB fs, so each bh linked to the request is 4kB in
size. You'll then have

	current_nr_sectors 8 (4096 >> 9)
	nr_sectors 32 (the four buffer heads)
	req->buffer == req->bh1->b_data

ps2esdi only processes one chunk of the time (looks at
current_nr_sectors for request and buffer size). Once you complete that
hunk and call end_request, your request will then look like this:

	req -> bh2 -> bh3 -> bh4
	current_nr_sectors 8
	nr_sectors 24
	req->buffer == req->bh2->b_data

and so it continues. This is the easy way to process requests. However,
if you can start I/O on more than one buffer at the time (scatter
gather), you could then setup your sg tables by browsing the entire
request buffer_head list and initiate I/O as needed.

Bigger requests on the queue, means more I/O in progress being possible.
There's no rule that you have to finish a request in one go, so even if
you can only handle eg 64 sectors per request with sg, you could do
just start I/O on as many segments as you can and simply don't dequeue
the request until it's completely done. So the max_sectors patch is
never really needed if you know what you are doing.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PS/2 Esdi patch #8
  2001-05-26 14:58       ` Jens Axboe
@ 2001-05-26 15:33         ` A Duston
  2001-05-26 15:40           ` Jens Axboe
  0 siblings, 1 reply; 8+ messages in thread
From: A Duston @ 2001-05-26 15:33 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Gortmaker, Paul, Andersen, Rasmus, linux-kernel

Jens Axboe wrote:
>
>  --snip--
>
>  and so it continues. This is the easy way to process requests. However,
>  if you can start I/O on more than one buffer at the time (scatter
>  gather), you could then setup your sg tables by browsing the entire
>  request buffer_head list and initiate I/O as needed.
>
>  Bigger requests on the queue, means more I/O in progress being possible.
>  There's no rule that you have to finish a request in one go, so even if
>  you can only handle eg 64 sectors per request with sg, you could do
>  just start I/O on as many segments as you can and simply don't dequeue
>  the request until it's completely done. So the max_sectors patch is
>  never really needed if you know what you are doing.

Can I still gain any advantage if the hardware can only have one I/O inflight
per device?  I am not sure the ps2esdi interface supports this.

Hal Duston
hald@sound.net



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: PS/2 Esdi patch #8
  2001-05-26 15:33         ` A Duston
@ 2001-05-26 15:40           ` Jens Axboe
  0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2001-05-26 15:40 UTC (permalink / raw)
  To: A Duston; +Cc: Gortmaker, Paul, Andersen, Rasmus, linux-kernel

On Sat, May 26 2001, A Duston wrote:
> >  and so it continues. This is the easy way to process requests. However,
> >  if you can start I/O on more than one buffer at the time (scatter
> >  gather), you could then setup your sg tables by browsing the entire
> >  request buffer_head list and initiate I/O as needed.
> >
> >  Bigger requests on the queue, means more I/O in progress being possible.
> >  There's no rule that you have to finish a request in one go, so even if
> >  you can only handle eg 64 sectors per request with sg, you could do
> >  just start I/O on as many segments as you can and simply don't dequeue
> >  the request until it's completely done. So the max_sectors patch is
> >  never really needed if you know what you are doing.
> 
> Can I still gain any advantage if the hardware can only have one I/O inflight
> per device?  I am not sure the ps2esdi interface supports this.

Not really, and especially for a slow interface like ps2esdi :-)

There's a small optimization possible for ps2esdi I see, but the chance
of it happening in real life is probably pretty slim. Even if you can't
do sg and you can't have more than one command pending, you could still
be lucky and do I/O to more than ->buffer provided that bh1 and bh2 etc
are contigous in memory. For a 4kB fs the chances are close to 0 that
this will happen once the system has been up for a little while (and
memory starts to fragment). For a 1kB fs the chances are probably
bigger.

If I were you, I'd leave it the way it is now. As long as you work on
current_nr_sectors, the only thing that Paul's patch will accomplish is
make the queue smaller. It will buy you nothing.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2001-05-27  0:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-05-23 22:54 PS/2 Esdi patch #8 Hal Duston
2001-05-24 20:46 ` Paul Gortmaker
2001-05-25 14:46   ` Jens Axboe
2001-05-26  9:53     ` Paul Gortmaker
2001-05-26 14:49     ` A Duston
2001-05-26 14:58       ` Jens Axboe
2001-05-26 15:33         ` A Duston
2001-05-26 15:40           ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).