[PATCH] Configure IDE probe delays

[PATCH] Configure IDE probe delays

[Todd Poynor]

IDE initialization and probing makes numerous calls to sleep for 50
milliseconds while waiting for the interface to return probe status and
such.  The value is probably rather conservative for modern hardware:
Tim Bird, David Singleton, and I have tried reducing this value to 1-5ms
on various systems (desktop, laptop, and embedded MIPS with PCI-to-IDE
bridge) with no observed ill effect (and delays in loops such as at
actual_try_to_identify() continued to deliver results after a single
delay in my trials).  But I haven't looked into ATA standards or
hardware specs enough to be very confident that these results would
apply to most or all of today's IDE hardware.

The 50ms delays (I counted 82 of 'em on my system) add up to about 4
seconds of kernel startup time on my laptop and Tim reports about 5
seconds on his workstation.  This is a more important matter for
consumer electronics devices, where these delays may constitute a large
fraction of the time required to get the gadget running.  Embedded
system designers have therefore asked for the ability to customize the
value for known hardware.  Tim discussed this during his OLS
presentation on improving boot times.

Any comments on this suggested patch that allows kernel command line
parameter ide-delay=2 to set the probing delay to 2ms, or any insight
into the risks involved in modifying this value?  Another possibility
would be to configure the value in the IDE interface and device drivers
according to known hardware characteristics.  Thanks.

--- linux-2.6.8-rc2-orig/drivers/ide/ide.c	2004-07-27 14:41:02.000000000 -0700
+++ linux-2.6.8-rc2-ide-delay/drivers/ide/ide.c	2004-07-29 18:38:59.000000000 -0700
@@ -196,6 +196,9 @@
 
 EXPORT_SYMBOL(ide_hwifs);
 
+int ide_delay = 50;			/* milliseconds */
+EXPORT_SYMBOL(ide_delay);
+
 extern ide_driver_t idedefault_driver;
 static void setup_driver_defaults(ide_driver_t *driver);
 
@@ -1766,6 +1769,12 @@
 	}
 #endif /* CONFIG_BLK_DEV_IDEPCI */
 
+	if (!strncmp(s, "ide-delay=", 10)) {
+		ide_delay = simple_strtoul(s+10,NULL,0);
+		printk(" : Delay set to %dms\n", ide_delay);
+		return 1;
+	}
+
 	/*
 	 * Look for drive options:  "hdx="
 	 */
--- linux-2.6.8-rc2-orig/drivers/ide/ide-iops.c	2004-07-27 14:28:26.000000000 -0700
+++ linux-2.6.8-rc2-ide-delay/drivers/ide/ide-iops.c	2004-07-29 18:07:49.000000000 -0700
@@ -30,6 +30,8 @@
 #include <asm/io.h>
 #include <asm/bitops.h>
 
+extern int ide_delay;
+
 /*
  *	Conventional PIO operations for ATA devices
  */
@@ -767,7 +769,7 @@
 	SELECT_MASK(drive, 1);
 	if (IDE_CONTROL_REG)
 		hwif->OUTB(drive->ctl,IDE_CONTROL_REG);
-	msleep(50);
+	msleep(ide_delay);
 	hwif->OUTB(WIN_IDENTIFY, IDE_COMMAND_REG);
 	timeout = jiffies + WAIT_WORSTCASE;
 	do {
@@ -775,9 +777,9 @@
 			SELECT_MASK(drive, 0);
 			return 0;	/* drive timed-out */
 		}
-		msleep(50);	/* give drive a breather */
+		msleep(ide_delay);	/* give drive a breather */
 	} while (hwif->INB(IDE_ALTSTATUS_REG) & BUSY_STAT);
-	msleep(50);	/* wait for IRQ and DRQ_STAT */
+	msleep(ide_delay);	/* wait for IRQ and DRQ_STAT */
 	if (!OK_STAT(hwif->INB(IDE_STATUS_REG),DRQ_STAT,BAD_R_STAT)) {
 		SELECT_MASK(drive, 0);
 		printk("%s: CHECK for good STATUS\n", drive->name);
@@ -827,7 +829,7 @@
 	u8 stat;
 
 //	while (HWGROUP(drive)->busy)
-//		msleep(50);
+//		msleep(ide_delay);
 
 #ifdef CONFIG_BLK_DEV_IDEDMA
 	if (hwif->ide_dma_check)	 /* check if host supports DMA */
--- linux-2.6.8-rc2-orig/drivers/ide/ide-probe.c	2004-07-27 14:28:26.000000000 -0700
+++ linux-2.6.8-rc2-ide-delay/drivers/ide/ide-probe.c	2004-07-29 18:59:47.000000000 -0700
@@ -56,6 +56,8 @@
 #include <asm/uaccess.h>
 #include <asm/io.h>
 
+extern int ide_delay;
+
 /**
  *	generic_id		-	add a generic drive id
  *	@drive:	drive to make an ID block for
@@ -273,7 +275,7 @@
 	u8 s = 0, a = 0;
 
 	/* take a deep breath */
-	msleep(50);
+	msleep(ide_delay);
 
 	if (IDE_CONTROL_REG) {
 		a = hwif->INB(IDE_ALTSTATUS_REG);
@@ -312,11 +314,11 @@
 			return 1;
 		}
 		/* give drive a breather */
-		msleep(50);
+		msleep(ide_delay);
 	} while ((hwif->INB(hd_status)) & BUSY_STAT);
 
 	/* wait for IRQ and DRQ_STAT */
-	msleep(50);
+	msleep(ide_delay);
 	if (OK_STAT((hwif->INB(IDE_STATUS_REG)), DRQ_STAT, BAD_R_STAT)) {
 		unsigned long flags;
 
@@ -445,15 +447,15 @@
 	/* needed for some systems
 	 * (e.g. crw9624 as drive0 with disk as slave)
 	 */
-	msleep(50);
+	msleep(ide_delay);
 	SELECT_DRIVE(drive);
-	msleep(50);
+	msleep(ide_delay);
 	if (hwif->INB(IDE_SELECT_REG) != drive->select.all && !drive->present) {
 		if (drive->select.b.unit != 0) {
 			/* exit with drive0 selected */
 			SELECT_DRIVE(&hwif->drives[0]);
 			/* allow BUSY_STAT to assert & clear */
-			msleep(50);
+			msleep(ide_delay);
 		}
 		/* no i/f present: mmm.. this should be a 4 -ml */
 		return 3;
@@ -476,14 +478,14 @@
 			printk("%s: no response (status = 0x%02x), "
 				"resetting drive\n", drive->name,
 				hwif->INB(IDE_STATUS_REG));
-			msleep(50);
+			msleep(ide_delay);
 			hwif->OUTB(drive->select.all, IDE_SELECT_REG);
-			msleep(50);
+			msleep(ide_delay);
 			hwif->OUTB(WIN_SRST, IDE_COMMAND_REG);
 			timeout = jiffies;
 			while (((hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) &&
 			       time_before(jiffies, timeout + WAIT_WORSTCASE))
-				msleep(50);
+				msleep(ide_delay);
 			rc = try_to_identify(drive, cmd);
 		}
 		if (rc == 1)
@@ -498,7 +500,7 @@
 	if (drive->select.b.unit != 0) {
 		/* exit with drive0 selected */
 		SELECT_DRIVE(&hwif->drives[0]);
-		msleep(50);
+		msleep(ide_delay);
 		/* ensure drive irq is clear */
 		(void) hwif->INB(IDE_STATUS_REG);
 	}
@@ -515,7 +517,7 @@
 
 	printk("%s: enabling %s -- ", hwif->name, drive->id->model);
 	SELECT_DRIVE(drive);
-	msleep(50);
+	msleep(ide_delay);
 	hwif->OUTB(EXABYTE_ENABLE_NEST, IDE_COMMAND_REG);
 	timeout = jiffies + WAIT_WORSTCASE;
 	do {
@@ -523,10 +525,10 @@
 			printk("failed (timeout)\n");
 			return;
 		}
-		msleep(50);
+		msleep(ide_delay);
 	} while ((hwif->INB(IDE_STATUS_REG)) & BUSY_STAT);
 
-	msleep(50);
+	msleep(ide_delay);
 
 	if (!OK_STAT((hwif->INB(IDE_STATUS_REG)), 0, BAD_STAT)) {
 		printk("failed (status = 0x%02x)\n", hwif->INB(IDE_STATUS_REG));
@@ -767,7 +769,7 @@
 		udelay(10);
 		hwif->OUTB(8, hwif->io_ports[IDE_CONTROL_OFFSET]);
 		do {
-			msleep(50);
+			msleep(ide_delay);
 			stat = hwif->INB(hwif->io_ports[IDE_STATUS_OFFSET]);
 		} while ((stat & BUSY_STAT) && time_after(timeout, jiffies));
 
--- linux-2.6.8-rc2-orig/Documentation/ide.txt	2004-07-27 14:29:10.000000000 -0700
+++ linux-2.6.8-rc2-ide-delay/Documentation/ide.txt	2004-07-29 18:33:44.000000000 -0700
@@ -304,6 +304,9 @@
 
  "ide=reverse"		: formerly called to pci sub-system, but now local.
 
+ "ide-delay=xx"		: set delay in milliseconds for initialization and
+			  probing.  Defaults to 50ms.
+
 The following are valid ONLY on ide0 (except dc4030), which usually corresponds
 to the first ATA interface found on the particular host, and the defaults for
 the base,ctl ports must not be altered.


-- 
Todd Poynor
MontaVista Software

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Fri, 2004-07-30 at 15:11, Todd Poynor wrote:
> Any comments on this suggested patch that allows kernel command line
> parameter ide-delay=2 to set the probing delay to 2ms, or any insight
> into the risks involved in modifying this value?  Another possibility
> would be to configure the value in the IDE interface and device drivers
> according to known hardware characteristics.  Thanks.

Even if the default stays at 50, this patch is an improvement because it
replaces a magic number with a named value, which is never a bad thing. 
I will let you know the results on my system.

Lee

Re: [PATCH] Configure IDE probe delays

[Alan Cox]

On Gwe, 2004-07-30 at 20:11, Todd Poynor wrote:
> IDE initialization and probing makes numerous calls to sleep for 50
> milliseconds while waiting for the interface to return probe status and
> such. 

Please make it taint the kernel if you do that so we can ignore all the
bug reports. That or justify it with a cite from the ATA standards ?

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Fri, 2004-07-30 at 18:35, Alan Cox wrote:
> On Gwe, 2004-07-30 at 20:11, Todd Poynor wrote:
> > IDE initialization and probing makes numerous calls to sleep for 50
> > milliseconds while waiting for the interface to return probe status and
> > such. 
> 
> Please make it taint the kernel if you do that so we can ignore all the
> bug reports. That or justify it with a cite from the ATA standards ?
> 

Works great on my hardware.  Well worth the savings in boot time.

Lee

Re: [PATCH] Configure IDE probe delays

[Jeff Garzik]

Alan Cox wrote:
> On Gwe, 2004-07-30 at 20:11, Todd Poynor wrote:
> 
>>IDE initialization and probing makes numerous calls to sleep for 50
>>milliseconds while waiting for the interface to return probe status and
>>such. 
> 
> 
> Please make it taint the kernel if you do that so we can ignore all the
> bug reports. That or justify it with a cite from the ATA standards ?


The 50 milliseconds is a pragmatic number you won't find in any 
standard.  Linux IDE driver does the "bang at the door" method of 
probing, which you won't find in any standard either :)

The ATA standard defines times for things like software reset (SRST), 
but Linux IDE driver takes a unique approach -- just issue down the 
first command (IDENTIFY DEVICE), and see what you get.  The command (or 
error) responses give you information that allows you to probe further.

The 50ms delay hides things like the device coming out of reset, the 
interface powering up on the first command issued to the device, etc.

IMO the number is a "change it at your own risk" number that shouldn't 
be touched unless the touch-er is also willing to rewrite the IDE 
driver's probe code ;-)

SATA is much faster at probing, and PATA will become progressively less 
common, so the motivation for changing the probe code is low, too.

	Jeff

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Sat, 2004-07-31 at 07:34, Denis Vlasenko wrote:
> On Saturday 31 July 2004 03:12, Lee Revell wrote:
> > On Fri, 2004-07-30 at 18:35, Alan Cox wrote:
> > > On Gwe, 2004-07-30 at 20:11, Todd Poynor wrote:
> > > > IDE initialization and probing makes numerous calls to sleep for 50
> > > > milliseconds while waiting for the interface to return probe status and
> > > > such.
> > >
> > > Please make it taint the kernel if you do that so we can ignore all the
> > > bug reports. That or justify it with a cite from the ATA standards ?
> >
> > Works great on my hardware.  Well worth the savings in boot time.
> 
> Crowd of "my old crapbox no longer boots with newer kernel, wtf?" people
> won't be happy at all.
> 
> +               ide_delay = simple_strtoul(s+10,NULL,0);
> +               printk(" : Delay set to %dms\n", ide_delay);
> +               return 1;
> 

I wonder if 83 probes are really necessary.  Maybe this could be
optimized a bit.

Lee

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Sat, 2004-07-31 at 07:34, Denis Vlasenko wrote:
> On Saturday 31 July 2004 03:12, Lee Revell wrote:
> > On Fri, 2004-07-30 at 18:35, Alan Cox wrote:
> > > On Gwe, 2004-07-30 at 20:11, Todd Poynor wrote:
> > > > IDE initialization and probing makes numerous calls to sleep for 50
> > > > milliseconds while waiting for the interface to return probe status and
> > > > such.
> > >
> > > Please make it taint the kernel if you do that so we can ignore all the
> > > bug reports. That or justify it with a cite from the ATA standards ?
> >
> > Works great on my hardware.  Well worth the savings in boot time.
> 
> Crowd of "my old crapbox no longer boots with newer kernel, wtf?" people
> won't be happy at all.

Maybe we need a CONFIG_ANCIENT_HARDWARE that people can set if they want
to use old stuff, and anywhere in the code we take a big hit to make
some ancient device work wouldn't get compiled.  Devices could be added
to this class as they are identified.

Lee 

Re: [PATCH] Configure IDE probe delays

[Alan Cox]

On Sad, 2004-07-31 at 19:06, Lee Revell wrote:
> Maybe we need a CONFIG_ANCIENT_HARDWARE that people can set if they want
> to use old stuff, and anywhere in the code we take a big hit to make
> some ancient device work wouldn't get compiled.  Devices could be added
> to this class as they are identified.

Wrong way around. You want a CONFIG_BOY_RACER option for people with
overclocked computers and delay loops "optimised" away.

If you want to speed this up then the two bits that the initial proposal
and Jeff have sensibly come up with are

1.	Are we doing too many probes
2.	Should we switch to proper reset polling

For certain cases (PPC spin up) we actually have switched to doing drive
spin up this way, I certainly have no objection to doing the rest of the
boot optimisation by following the standards carefully.

BTW: We should also turn on the PPC specific spin up stuff generically
nowdays with suspend/resume and stuff like LinuxBIOS.

Alan

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Sat, 2004-07-31 at 15:35, Alan Cox wrote:
> On Sad, 2004-07-31 at 19:06, Lee Revell wrote:
> > Maybe we need a CONFIG_ANCIENT_HARDWARE that people can set if they want
> > to use old stuff, and anywhere in the code we take a big hit to make
> > some ancient device work wouldn't get compiled.  Devices could be added
> > to this class as they are identified.
> 
> Wrong way around. You want a CONFIG_BOY_RACER option for people with
> overclocked computers and delay loops "optimised" away.
> 

LOL.  But, the last overclocking I did was when i figured out that
removing the 'SPEED' jumper on my 486/33 would make it run at 50Mhz.  I
am actually working on an embedded system (well, small anyway).  I have
never used Gentoo.

Even if it's not appropriate for this case, there have to be some places
in the kernel where this would be useful.  What about hardware that is 
broken, requiring a device-specific kludge?  Hardware that the kernel
developers would prefer didn't exist.  There have to be some of these. 
Or are most of these already broken out and disabled by default like the
old CMD640 ide bug?

Lee

Re: [PATCH] Configure IDE probe delays

[Jeff Garzik]

Lee Revell wrote:
> Even if it's not appropriate for this case, there have to be some places
> in the kernel where this would be useful.  What about hardware that is 
> broken, requiring a device-specific kludge?  Hardware that the kernel
> developers would prefer didn't exist.  There have to be some of these. 
> Or are most of these already broken out and disabled by default like the
> old CMD640 ide bug?


Broken hardware will always exist.  Sounds like you want 
CONFIG_PERFECT_WORLD ?

	Jeff

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Sat, 2004-07-31 at 17:44, Jeff Garzik wrote:
> Lee Revell wrote:
> > Even if it's not appropriate for this case, there have to be some places
> > in the kernel where this would be useful.  What about hardware that is 
> > broken, requiring a device-specific kludge?  Hardware that the kernel
> > developers would prefer didn't exist.  There have to be some of these. 
> > Or are most of these already broken out and disabled by default like the
> > old CMD640 ide bug?
> 
> 
> Broken hardware will always exist.  Sounds like you want 
> CONFIG_PERFECT_WORLD ?
> 

Sure.  Got a patch?

Lee

Re: [PATCH] Configure IDE probe delays

[Alan Cox]

On Llu, 2004-08-02 at 22:56, Tim Bird wrote:
> I'm still considering Jeff's comment, and I'll respond to that separately.

If it's done properly everyone wins, and I suspect if you are using
things like CF you will win big time using the PPC spin up delay and not
waiting for disk spin up in the boot firmware.

Re: [PATCH] Configure IDE probe delays

[Tim Bird]

Denis Vlasenko wrote:
 > Crowd of "my old crapbox no longer boots with newer kernel, wtf?" people
 > won't be happy at all.
 >
 > +               ide_delay = simple_strtoul(s+10,NULL,0);
 > +               printk(" : Delay set to %dms\n", ide_delay);
 > +               return 1;
 >
 > Can we have a bit louder warning here?

We can make this even stronger if desired.

Alan Cox wrote:
> On Gwe, 2004-07-30 at 20:11, Todd Poynor wrote:
> 
>>IDE initialization and probing makes numerous calls to sleep for 50
>>milliseconds while waiting for the interface to return probe status and
>>such. 
> 
> 
> Please make it taint the kernel if you do that so we can ignore all the
> bug reports.

We can certainly make the warning louder as Denis suggests, and also
make the wording in the documentation stronger, like so...

+ "ide-delay=xx"		: set delay in milliseconds for initialization and
+			  probing.  Defaults to 50ms.  Use at your own risk.  Please
+			  do not report ANY problems to kernel developers if using a
+			  non-default setting.

If needed, I'd be willing to taint the kernel to reduce the burden
of related bug reports.  I'm a little concerned that tainting conveys
a meaning of GPL conformance questionability, which would not be the
case here. (Just configuration questionability...  ;-)

This is primarily to support embedded systems with known hardware, but
I *DO* see the need to avoid having desktop people experiment with it
and make themselves (and by transitivity kernel developers) unhappy.

This is, admittedly, introducing a gun which one might use to shoot
one's own foot.  However, in my testing these delays accounted
for about 70% of total kernel bootup time, and this is a pretty easy
way to "fix" it.  I know many companies are very willing to bear
the burden of testing this out on their hardware before shipping
products (Sony would), in exchange for the dramatic bootup time
savings.

I'm still considering Jeff's comment, and I'll respond to that separately.

=============================
Tim Bird
Architecture Group Co-Chair, CE Linux Forum
Senior Staff Engineer, Sony Electronics
E-mail: tim.bird@am.sony.com
=============================

Re: [PATCH] Configure IDE probe delays

[Mark Lord]

I'm the dude responsible for the infamous "50 milliseconds" here.

I agree that (1) it is overkill, (2) it could be optimised,
and (3) it is very very non-standard.

But it also works extraordinarilly well.  I still am very active
with ATA and SATA driver development, and the basic Linux IDE probe
works for me on vendor hardware where their own standards-specific
routines sometimes fail (even in their windows drivers).

If possible, it would be best to let it be, and over time it will
be less and less important as SATA and kin take over the universe.

Every time I tried to tweak it to be faster, it seemed to break
some system or another.

One possibility here would be to augment it with reset signature probing,
and/or a cyl-high read/write test.  These could speed things up for
more mainstream cases.  But I'm not going to touch what's there myself!

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: [PATCH] Configure IDE probe delays

[Anthony de Boer]

Jeff Garzik wrote:
> Broken hardware will always exist.  Sounds like you want 
> CONFIG_PERFECT_WORLD ?

You've already got #ifndef I_WISH_WORLD_WERE_PERFECT in net/ipv4/ip_gre.c
and ipip.c, and should likely maintain compatibility with what's already
there.

-- 
Anthony de Boer

Re: [PATCH] Configure IDE probe delays

[Tim Bird]

Alan Cox wrote:
> If you want to speed this up then the two bits that the initial proposal
> and Jeff have sensibly come up with are
> 
> 1.	Are we doing too many probes

By way of commentary, it *is* possible to reduce the number of probes
using existing kernel command line options (hd<x>=noprobe and
ide<x>=noprobe)  This helps on systems where interfaces or devices
are known not to exist.  This is described in my OLS paper, and I
plan to put up a wiki page with instructions for interested parties,
real soon now... :-)

However, further reducing the number of probes is still a worthy goal.
We'll take a look at this when we get a chance.  Given some
of the feedback on this thread, this sounds like it might be prone
to worse breakage for legacy hardware than simply adjusting the delay
duration.

> 2.	Should we switch to proper reset polling
> 
> For certain cases (PPC spin up) we actually have switched to doing drive
> spin up this way...

I'm not sure what this means.  Can someone tell me more about this
or point me to some code?  (Sorry for my ignorance, I'm not an IDE
expert by any means.)

BTW - Any comments on Todd's new patch? (see message with title
"IDE probe delay symbol")  This one doesn't make the value configurable,
but does use a #define (preserving the traditional 50 ms value).  This
at least removes a magic number, and it makes it easier to identify the
msleeps that have historically been related.

With this new patch, nothing changes for the legacy crowd, but it still
makes it easier for RACER_BOYs ;-) to dink with the value.

=============================
Tim Bird
Architecture Group Co-Chair, CE Linux Forum
Senior Staff Engineer, Sony Electronics
E-mail: tim.bird@am.sony.com
=============================

Re: [PATCH] Configure IDE probe delays

[Jeff Garzik]

Mark Lord wrote:
> I'm the dude responsible for the infamous "50 milliseconds" here.
> 
> I agree that (1) it is overkill, (2) it could be optimised,
> and (3) it is very very non-standard.
> 
> But it also works extraordinarilly well.  I still am very active
> with ATA and SATA driver development, and the basic Linux IDE probe
> works for me on vendor hardware where their own standards-specific
> routines sometimes fail (even in their windows drivers).
> 
> If possible, it would be best to let it be, and over time it will
> be less and less important as SATA and kin take over the universe.

Honestly, this is what I would prefer:  leave drivers/ide probing alone. 
    As we say in the South, "if it ain't broke, don't fix it"

Long term, migrate to libata which should provide quite rapid PATA probing.


> One possibility here would be to augment it with reset signature probing,
> and/or a cyl-high read/write test.  These could speed things up for
> more mainstream cases.  But I'm not going to touch what's there myself!

libata already does this :)

	Jeff

Re: [PATCH] Configure IDE probe delays

[Tim Bird]

Jeff Garzik wrote:
> Honestly, this is what I would prefer:  leave drivers/ide probing alone. 
>    As we say in the South, "if it ain't broke, don't fix it"
> 
> Long term, migrate to libata which should provide quite rapid PATA probing.

Thanks.  This is helpful information.  I will check into libata.

With regard to brokenness, for many consumer products, there is a
strong desire that they boot in under 1 second.  The general
agreement among CELF members is that we'd like to see the
kernel booting in under 500 milliseconds.  This is just not
possible with the current long IDE probes.

Our current testing seems to indicate that shortened delays
will not be a problem for the hardware being considered for
use in current CE products.  I can appreciate that shortened
delays are problematical for the wider kernel audience.
(I feel compelled to point out that the current patch
doesn't change the delays.)

It appears that this change won't be accepted.  That's OK.  We'll
maintain a patch at CELF for interested parties, and try to document
the caveats from this thread (and from additional testing) to try
to avoid problems.  Also, we'll check into the other mechanisms
for probe reduction discussed in this thread, and come back if we
find some promising bits.

Thanks for everyone that responded.

=============================
Tim Bird
Architecture Group Co-Chair, CE Linux Forum
Senior Staff Engineer, Sony Electronics
E-mail: tim.bird@am.sony.com
=============================

Re: [PATCH] Configure IDE probe delays

[Todd Poynor]

At the risk of flogging this horse after the death certificate was just 
signed, there were two threads running through the discussion and I 
wasn't sure where things stand.  One thread, as I read it, was: "Don't 
hack around the delays, let's change the probing and do it right".  The 
other thread was: "Don't mess around with IDE probing, leave that alone 
and let's do it right for SATA".

If the latter route is how things are headed then I'd still suggest 
replacing the scattered hardcoded "50" constants with a meaningful symbol.

If, on the other hand, IDE probe delays might someday become more 
complicated, such that a single symbol is not sufficient (as Alan warned 
against), then at least it would be easier to find 'em all.  And 
converting some of the calls to use a new symbol for a new class of 
delays would be quite easy.

Anyhow, not a big deal; if there's anything I can help out with to 
improve the situation then I'd be glad to do so.

-- 
Todd Poynor
MontaVista Software

Re: [PATCH] Configure IDE probe delays

[Greg Stark]

Lee Revell <rlrevell@joe-job.com> writes:

> I wonder if 83 probes are really necessary.  Maybe this could be
> optimized a bit.

Or if the kernel could be doing something useful during that time. I don't
suppose it's possible to probe two different ide interfaces at the same time,
is it?


-- 
greg

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Fri, 2004-08-27 at 13:45, Greg Stark wrote:
> Lee Revell <rlrevell@joe-job.com> writes:
> 
> > I wonder if 83 probes are really necessary.  Maybe this could be
> > optimized a bit.
> 
> Or if the kernel could be doing something useful during that time. I don't
> suppose it's possible to probe two different ide interfaces at the same time,
> is it?
> 

Did the patch to move this into a #define ever get merged?  Seems like a
no brainer, as it eliminates a magic number.

Lee

Re: [PATCH] Configure IDE probe delays

[Bartlomiej Zolnierkiewicz]

On Friday 27 August 2004 19:53, Lee Revell wrote:
> On Fri, 2004-08-27 at 13:45, Greg Stark wrote:
> > Lee Revell <rlrevell@joe-job.com> writes:
> > > I wonder if 83 probes are really necessary.  Maybe this could be
> > > optimized a bit.
> >
> > Or if the kernel could be doing something useful during that time. I
> > don't suppose it's possible to probe two different ide interfaces at the
> > same time, is it?
>
> Did the patch to move this into a #define ever get merged?  Seems like a
> no brainer, as it eliminates a magic number.

No and it won't because it is not a 'magic number' but rather a 'random 
number' (see Alan's mail for explanation).

BTW Lee, 48-bit addressing doesn't mean that capacity > 137GB

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Fri, 2004-08-27 at 14:05, Bartlomiej Zolnierkiewicz wrote:
> On Friday 27 August 2004 19:53, Lee Revell wrote:
> > On Fri, 2004-08-27 at 13:45, Greg Stark wrote:
> > > Lee Revell <rlrevell@joe-job.com> writes:
> > > > I wonder if 83 probes are really necessary.  Maybe this could be
> > > > optimized a bit.
> > >
> > > Or if the kernel could be doing something useful during that time. I
> > > don't suppose it's possible to probe two different ide interfaces at the
> > > same time, is it?
> >
> > Did the patch to move this into a #define ever get merged?  Seems like a
> > no brainer, as it eliminates a magic number.
> 
> No and it won't because it is not a 'magic number' but rather a 'random 
> number' (see Alan's mail for explanation).
> 

OK, sorry.  Missed that one the first time.

> BTW Lee, 48-bit addressing doesn't mean that capacity > 137GB
> 

What determines whether 48 bit addressing will be used then?

Lee

Re: [PATCH] Configure IDE probe delays

[Bartlomiej Zolnierkiewicz]

On Friday 27 August 2004 20:08, Lee Revell wrote:
> On Fri, 2004-08-27 at 14:05, Bartlomiej Zolnierkiewicz wrote:
> > On Friday 27 August 2004 19:53, Lee Revell wrote:
> > > On Fri, 2004-08-27 at 13:45, Greg Stark wrote:
> > > > Lee Revell <rlrevell@joe-job.com> writes:
> > > > > I wonder if 83 probes are really necessary.  Maybe this could be
> > > > > optimized a bit.
> > > >
> > > > Or if the kernel could be doing something useful during that time. I
> > > > don't suppose it's possible to probe two different ide interfaces at
> > > > the same time, is it?
> > >
> > > Did the patch to move this into a #define ever get merged?  Seems like
> > > a no brainer, as it eliminates a magic number.
> >
> > No and it won't because it is not a 'magic number' but rather a 'random
> > number' (see Alan's mail for explanation).
>
> OK, sorry.  Missed that one the first time.
>
> > BTW Lee, 48-bit addressing doesn't mean that capacity > 137GB

Contrary is true of course:
if capacity > 137 GB then disk supports 48-bit addressing.

> What determines whether 48 bit addressing will be used then?

Availability of 48-bit addressing feature set and host capabilities
(some don't support LBA48 when DMA is used etc.).

Re: [PATCH] Configure IDE probe delays

[Mark Lord]

Bartlomiej Zolnierkiewicz wrote:
>
>>What determines whether 48 bit addressing will be used then?
> 
> Availability of 48-bit addressing feature set and host capabilities
> (some don't support LBA48 when DMA is used etc.).

I haven't examined the "released" IDE drivers in some time,
but one optimisation that can save a LOT of CPU usage
is for the driver to only use LBA48 *when necessary*,
and use LBA28 I/O otherwise.

Each access to an IDE register typically chews up 600+ns,
or the equivalent of a couple thousand instruction executions
on a modern core.  Avoiding LBA48 when it's not needed will
save four such accesses per I/O, or about 2.5us.

LBA48 is only needed when (1) the sector count is greater than 256,
and/or (2) the ending sector number >= (1<<28).

I regularly include this optimisation in the drivers I have been
working on since LBA48 first appeared.

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: [PATCH] Configure IDE probe delays

[Jeff Garzik]

Mark Lord wrote:
> Bartlomiej Zolnierkiewicz wrote:
> 
>>
>>> What determines whether 48 bit addressing will be used then?
>>
>>
>> Availability of 48-bit addressing feature set and host capabilities
>> (some don't support LBA48 when DMA is used etc.).
> 
> 
> I haven't examined the "released" IDE drivers in some time,
> but one optimisation that can save a LOT of CPU usage
> is for the driver to only use LBA48 *when necessary*,
> and use LBA28 I/O otherwise.
> 
> Each access to an IDE register typically chews up 600+ns,
> or the equivalent of a couple thousand instruction executions
> on a modern core.  Avoiding LBA48 when it's not needed will
> save four such accesses per I/O, or about 2.5us.


Doing this is either pointless or impossible on newer SATA controllers. 
  Most are memory-mapped I/O not PIO, where the high-order bits of the 
ATA taskfile are accessed due to an extended register size, not 
"double-pumping" a FIFO.

Even-newer SATA controllers are FIS-based rather than taskfile-based, so 
you pass it a FIS (containing all the registers) unconditionally.

	Jeff

Re: [PATCH] Configure IDE probe delays

[Alan Cox]

On Mer, 2004-09-01 at 14:20, Mark Lord wrote:
> LBA48 is only needed when (1) the sector count is greater than 256,
> and/or (2) the ending sector number >= (1<<28).

I've played with this a bit and in the -ac IDE code it can drop back
to LBA28 for devices that are small enough not to need LBA48 when the
controller only supports PIO for LBA48 modes (eg some ALi) as 2.4-ac
did. 

> I regularly include this optimisation in the drivers I have been
> working on since LBA48 first appeared.

It isn't always a win. You get cut down to 256 sectors per I/O which for
some workloads has a cost and you need to factor that into the command
issue choice as well as the last sector number being accessed.

Alan

Re: [PATCH] Configure IDE probe delays

[Mark Lord]

 >> I haven't examined the "released" IDE drivers in some time,
 >> but one optimisation that can save a LOT of CPU usage
 >> is for the driver to only use LBA48 *when necessary*,
 >> and use LBA28 I/O otherwise.
 >>
 >> Each access to an IDE register typically chews up 600+ns,
 >> or the equivalent of a couple thousand instruction executions
 >> on a modern core.  Avoiding LBA48 when it's not needed will
 >> save four such accesses per I/O, or about 2.5us.

Yup, this is for ATA register accesses, not SATA FISs.
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Re: [PATCH] Configure IDE probe delays

[Mark Lord]

 > Doing this is either pointless or impossible on newer SATA controllers.
 >  Most are memory-mapped I/O not PIO, where the high-order bits of the
 > ATA taskfile are accessed due to an extended register size, not
 > "double-pumping" a FIFO.
 >
 > Even-newer SATA controllers are FIS-based rather than taskfile-based, so
 > you pass it a FIS (containing all the registers) unconditionally.

PCI accesses are not free, so anything that avoids having to
go over the PCI bus is a worthwhile optimization.

The processor buses run at 200-800Mhz or so, whereas PCI is normally
only clocking at 33Mhz, sometimes at 66Mhz.

With good ADMA or host-queuing controllers that access system
memory directly for their command blocks, then there's not much
(if any) penalty for the extra LBA48 setup.  But for "normal"
controllers (if such a beast even exists), the extra writes across
the PCI bus can be costly.

Hardware write-buffer FIFOs between the CPU and the PCI bus
can reduce the impact of this somewhat, but they are often
only 2-4 entries deep, and will be filled by a normal (S)ATA
command setup sequence.

This is one of those finer points that is very difficult to measure,
since the I/O throughput is pretty much unaffected by it.  But CPU
cycle count per-I/O setup is one way to measure it.

Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")

Jeff Garzik wrote:
> Mark Lord wrote:
> 
>> Bartlomiej Zolnierkiewicz wrote:
>>
>>>
>>>> What determines whether 48 bit addressing will be used then?
>>>
>>>
>>>
>>> Availability of 48-bit addressing feature set and host capabilities
>>> (some don't support LBA48 when DMA is used etc.).
>>
>>
>>
>> I haven't examined the "released" IDE drivers in some time,
>> but one optimisation that can save a LOT of CPU usage
>> is for the driver to only use LBA48 *when necessary*,
>> and use LBA28 I/O otherwise.
>>
>> Each access to an IDE register typically chews up 600+ns,
>> or the equivalent of a couple thousand instruction executions
>> on a modern core.  Avoiding LBA48 when it's not needed will
>> save four such accesses per I/O, or about 2.5us.
> 
> 
> 
> Doing this is either pointless or impossible on newer SATA controllers. 
>  Most are memory-mapped I/O not PIO, where the high-order bits of the 
> ATA taskfile are accessed due to an extended register size, not 
> "double-pumping" a FIFO.
> 
> Even-newer SATA controllers are FIS-based rather than taskfile-based, so 
> you pass it a FIS (containing all the registers) unconditionally.
> 
>     Jeff
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Configure IDE probe delays

[Bartlomiej Zolnierkiewicz]

Jens come with this long time ago but IDE driver is not ready for it,
there are some gaps to fill first.

Anyway, thanks for reminding me about this.

Cheers.

On Wednesday 01 September 2004 15:20, Mark Lord wrote:
> Bartlomiej Zolnierkiewicz wrote:
> >
> >>What determines whether 48 bit addressing will be used then?
> > 
> > Availability of 48-bit addressing feature set and host capabilities
> > (some don't support LBA48 when DMA is used etc.).
> 
> I haven't examined the "released" IDE drivers in some time,
> but one optimisation that can save a LOT of CPU usage
> is for the driver to only use LBA48 *when necessary*,
> and use LBA28 I/O otherwise.
>
> Each access to an IDE register typically chews up 600+ns,
> or the equivalent of a couple thousand instruction executions
> on a modern core.  Avoiding LBA48 when it's not needed will
> save four such accesses per I/O, or about 2.5us.
> 
> LBA48 is only needed when (1) the sector count is greater than 256,
> and/or (2) the ending sector number >= (1<<28).
> 
> I regularly include this optimisation in the drivers I have been
> working on since LBA48 first appeared.
> 
> Cheers

Re: [PATCH] Configure IDE probe delays

[Alan Cox]

On Mer, 2004-09-01 at 20:36, Lee Revell wrote:
> The effect can be measured using a recent version of the voluntary
> preemption patches, and disabling hardirq preemption.  In this situation
> the IDE I/O completion is by far the longest non-preemptible code path,
> so can be easily profiled from the latency traces.

A lot of IDE controllers hold off the CPU for long times when you do I/O
cycles. The other factor is PIO which defaults to IRQ masking for safety
on old controllers. For PCI we should probably default the other way.

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Wed, 2004-09-01 at 11:06, Alan Cox wrote:
> On Mer, 2004-09-01 at 14:20, Mark Lord wrote:
> > LBA48 is only needed when (1) the sector count is greater than 256,
> > and/or (2) the ending sector number >= (1<<28).
> 
> I've played with this a bit and in the -ac IDE code it can drop back
> to LBA28 for devices that are small enough not to need LBA48 when the
> controller only supports PIO for LBA48 modes (eg some ALi) as 2.4-ac
> did. 
> 
> > I regularly include this optimisation in the drivers I have been
> > working on since LBA48 first appeared.
> 
> It isn't always a win. You get cut down to 256 sectors per I/O which for
> some workloads has a cost and you need to factor that into the command
> issue choice as well as the last sector number being accessed.
> 

I have never been able to measure a decrease in disk throughput in any
disk benchmark with 256 sectors per I/O vs. 1024.  This is a modestly
powered desktop with a single drive though.  What kinds of workloads
would you expect to be affected by this?

Lee 

Re: [PATCH] Configure IDE probe delays

[Lee Revell]

On Wed, 2004-09-01 at 11:36, Mark Lord wrote:
> With good ADMA or host-queuing controllers that access system
> memory directly for their command blocks, then there's not much
> (if any) penalty for the extra LBA48 setup.  But for "normal"
> controllers (if such a beast even exists), the extra writes across
> the PCI bus can be costly.
> 
> Hardware write-buffer FIFOs between the CPU and the PCI bus
> can reduce the impact of this somewhat, but they are often
> only 2-4 entries deep, and will be filled by a normal (S)ATA
> command setup sequence.
> 
> This is one of those finer points that is very difficult to measure,
> since the I/O throughput is pretty much unaffected by it.  But CPU
> cycle count per-I/O setup is one way to measure it.
> 

The effect can be measured using a recent version of the voluntary
preemption patches, and disabling hardirq preemption.  In this situation
the IDE I/O completion is by far the longest non-preemptible code path,
so can be easily profiled from the latency traces.

Lee

Re: [PATCH] Configure IDE probe delays

[Mark Lord]

I agree that  >256 sectors will be a win, worth the
extra overhead because it will save subsequent overheads
of extra commands.

Which is why my original text (quoted by Alan) said:

On Mer, 2004-09-01 at 14:20, Mark Lord wrote:

 >> LBA48 is only needed when (1) the sector count is greater than 256,
 >> and/or (2) the ending sector number >= (1<<28).


Cheers
-- 
Mark Lord
(hdparm keeper & the original "Linux IDE Guy")