[patch 0/2] fastboot: allow skipping probing of known-unused ports in libata

[patch 0/2] fastboot: allow skipping probing of known-unused ports in libata

[Arjan van de Ven]

Hi,

if you KNOW a certain ata port is not connected (for example in a
netbook where there's just nothing there possible), we still probe
the port currently and waste time. This time is a significant part of
the total boot time.

This patch series introduces a module parameter that can be used
to tell the kernel which ports to skip, and the second patch
adds a DMI list for known setups so that for known cases
users don't have to add any parameters at all.

[patch 1/2] fastboot: Add a module parameter to skip probing of specific ports

[Arjan van de Ven]

From: Kristen Accardi <kristen.c.accardi@intel.com>
Subject: [PATCH] libata: Add a module parameter to skip probing of specific ports

Port probing by libata can easily take 10% or more of the kernel boot
time (2%+ of total). For cases where one knows there is nothing
connected to certain ports (for example on netbooks) this is a waste
of boot time.

This patch adds a module parameter that allows the admin to specify
to skip ports (specified by a bitmask) and recoup this boot time.
This capability is potentially also useful to get systems to boot
for cases where port-probing on a certain ports causes crashes.

A follow-on patch will add the capability to use DMI identification
to automate this for certain known systems.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/ata/libata-core.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 5ba96c5..831a8ca 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -148,6 +148,10 @@ static int ata_probe_timeout;
 module_param(ata_probe_timeout, int, 0444);
 MODULE_PARM_DESC(ata_probe_timeout, "Set ATA probing timeout (seconds)");
 
+static int libata_disable_ports;
+module_param_named(disable_ports, libata_disable_ports, int, 0444);
+MODULE_PARM_DESC(disable_ports, "Disable specified ports (as bitmask) at boot time");
+
 int libata_noacpi = 0;
 module_param_named(noacpi, libata_noacpi, int, 0444);
 MODULE_PARM_DESC(noacpi, "Disables the use of ACPI in probe/suspend/resume when set");
@@ -5651,6 +5655,13 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht)
 	for (i = 0; i < host->n_ports; i++) {
 		struct ata_port *ap = host->ports[i];
 
+		/*
+		 * check if the port is specified as 'to disable' by either
+		 * a module parameter or a DMI match
+		 */
+		if (libata_disable_ports & (1 << ap->port_no))
+			continue;
+
 		/* probe */
 		if (ap->ops->error_handler) {
 			struct ata_eh_info *ehi = &ap->link.eh_info;
-- 
1.5.5.1

Re: [patch 1/2] fastboot: Add a module parameter to skip probing of specific ports

[Alan Cox]

On Mon, 11 Aug 2008 15:36:41 -0700
Arjan van de Ven <arjan@infradead.org> wrote:

> 
> From: Kristen Accardi <kristen.c.accardi@intel.com>
> Subject: [PATCH] libata: Add a module parameter to skip probing of specific ports
> 
> Port probing by libata can easily take 10% or more of the kernel boot
> time (2%+ of total). For cases where one knows there is nothing
> connected to certain ports (for example on netbooks) this is a waste
> of boot time.
> 
> This patch adds a module parameter that allows the admin to specify
> to skip ports (specified by a bitmask) and recoup this boot time.
> This capability is potentially also useful to get systems to boot
> for cases where port-probing on a certain ports causes crashes.
> 
> A follow-on patch will add the capability to use DMI identification
> to automate this for certain known systems.

What happens if I plug in an additional libata using device ? What
defines the probe order here particularly as people are pushing for
parallel probing of multiple devices.

This doesn't appear to make any rational sense as the mask isn't tied to
the actual bus device identifier to keep it on the same port. It might
kidn of work for an EEEPC (until you see what people retrofit into the
corners of them) but it isn't a valid general solution.

Also the EEE problem seems to be a controller specific screwup - they
didn't apparently manage the enablebits on the ATA controller correctly,
so it belongs in that driver. That also lets you tie it to the right
system, pci id, bus id so it'll always hit the right device.

(Plus double check the enables code in case you are papering over the
real bug)

Second problem is you've changed the API. Several drivers do things on
the port 0 register they know they will receive and will simply crash and
burn if you change this.

NAK this patch for now: right theory, wrong implementation. Please post a
version which uses DMI in the relevant driver and checks the PCI DEVFN
matches.

Re: [patch 1/2] fastboot: Add a module parameter to skip probing of specific ports

[Arjan van de Ven]

On Tue, 12 Aug 2008 08:39:03 +0100
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> On Mon, 11 Aug 2008 15:36:41 -0700
> Arjan van de Ven <arjan@infradead.org> wrote:
> 
> > 
> > From: Kristen Accardi <kristen.c.accardi@intel.com>
> > Subject: [PATCH] libata: Add a module parameter to skip probing of
> > specific ports
> > 
> > Port probing by libata can easily take 10% or more of the kernel
> > boot time (2%+ of total). For cases where one knows there is nothing
> > connected to certain ports (for example on netbooks) this is a waste
> > of boot time.
> > 
> > This patch adds a module parameter that allows the admin to specify
> > to skip ports (specified by a bitmask) and recoup this boot time.
> > This capability is potentially also useful to get systems to boot
> > for cases where port-probing on a certain ports causes crashes.
> > 
> > A follow-on patch will add the capability to use DMI identification
> > to automate this for certain known systems.
> 
> What happens if I plug in an additional libata using device ?

on an eeepc ? ;)

at least for the command line option the owner (or whoever sets up
grub) knows the order and could adjust. For the DMI patch you do have a
point.

> What
> defines the probe order here particularly as people are pushing for
> parallel probing of multiple devices.

the patches I'm pushing for this don't change probe order; that has
been tried before and wasn't a great success.


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 1/2] fastboot: Add a module parameter to skip probing of specific ports

[Alan Cox]

> > What
> > defines the probe order here particularly as people are pushing for
> > parallel probing of multiple devices.
> 
> the patches I'm pushing for this don't change probe order; that has
> been tried before and wasn't a great success.

That was a question sorry. You are sledgehammering controllers by
discovery sequence - you've no idea if they will always be found in that
order. As such your boot option is incredibly fragile.

Re: [patch 1/2] fastboot: Add a module parameter to skip probing of specific ports

[Arjan van de Ven]

On Tue, 12 Aug 2008 13:00:27 +0100
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > > What
> > > defines the probe order here particularly as people are pushing
> > > for parallel probing of multiple devices.
> > 
> > the patches I'm pushing for this don't change probe order; that has
> > been tried before and wasn't a great success.
> 
> That was a question sorry. You are sledgehammering controllers by
> discovery sequence - you've no idea if they will always be found in
> that order. As such your boot option is incredibly fragile.

ok to answer your question;
today the probe order is consistent, at least on netbooks.
The admin or his installation program knows it when he has this (and if
linux were to grow full parallel probing it'll be optional, and if the
admin wants a fast boot he'll disable the parallel, reordering probe)
and can add the option for this case. It's the "push policy out of the
kernel" thing.. while the kernel probably can't know this (and you're
right, the DMI patch is thus the wrong thing to do).. the admin can.



-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 1/2] fastboot: Add a module parameter to skip probing of specific ports

[Alan Cox]

> and can add the option for this case. It's the "push policy out of the
> kernel" thing.. while the kernel probably can't know this (and you're
> right, the DMI patch is thus the wrong thing to do).. the admin can.

libata lets you set the nth controller to various things (40wire etc) so
I suspect the right thing to extent is the existing libata option code
that Tejun wrote to include 'skip' if it doesn't already.

If that gets done cc me and I'll then go audit the drivers to see who
apart from it821x knows port 0 is always probed

[patch 2/2] fastboot: use a DMI match table to set defaults for port-disable

[Arjan van de Ven]

From: Arjan van de Ven <arjan@linux.intel.com>
Subject: [PATCH] libata: use a DMI match table to set defaults for port-disable

As suggested by Jeff:
This patch adds a DMI table that is used for setting defaults for
the new dont-probe-port parameter boot time optimization feature.

Initially, the table only contains the Asus EeePC 901 as this is the
device I have for testing.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 drivers/ata/libata-core.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 831a8ca..a6b0540 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -56,6 +56,7 @@
 #include <linux/workqueue.h>
 #include <linux/scatterlist.h>
 #include <linux/io.h>
+#include <linux/dmi.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_host.h>
@@ -166,6 +167,37 @@ MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 
 
+static int __init dmi_disable_ports(const struct dmi_system_id *d)
+{
+	int value = (unsigned long)d->driver_data;
+
+	printk(KERN_INFO "libata: DMI match used to disable probing\n");
+	libata_disable_ports |= value;
+	return 0;
+}
+
+#define DMI_PORT_ATA1 (void *)0x1
+#define DMI_PORT_ATA2 (void *)0x2
+#define DMI_PORT_ATA3 (void *)0x4
+#define DMI_PORT_ATA4 (void *)0x8
+
+static struct dmi_system_id libata_dmi_table[] __initdata = {
+	/*
+	 * The Asus EeePC901 has its devices attached to ATA2 only
+	 */
+	{
+	.callback = dmi_disable_ports,
+	.driver_data = DMI_PORT_ATA1,
+	.ident = "Asus EeePC 901",
+	.matches = {
+		     DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK Computer INC."),
+		     DMI_MATCH(DMI_PRODUCT_NAME, "901"),
+		},
+	},
+	{}
+};
+
+
 /**
  *	ata_force_cbl - force cable type according to libata.force
  *	@ap: ATA port of interest
@@ -6126,6 +6158,7 @@ static void __init ata_parse_force_param(void)
 static int __init ata_init(void)
 {
 	ata_parse_force_param();
+	dmi_check_system(libata_dmi_table);
 
 	ata_wq = create_workqueue("ata");
 	if (!ata_wq)
-- 
1.5.5.1

Re: [patch 2/2] fastboot: use a DMI match table to set defaults for port-disable

[Marcel Holtmann]

Hi Arjan,

> Subject: [PATCH] libata: use a DMI match table to set defaults for port-disable
> 
> As suggested by Jeff:
> This patch adds a DMI table that is used for setting defaults for
> the new dont-probe-port parameter boot time optimization feature.
> 
> Initially, the table only contains the Asus EeePC 901 as this is the
> device I have for testing.

last time I had mine open, I saw that the other ATA connectors are in
theory usable. You have to do some heavy modifications, but some people
do modifications to this kind of hardware to use it for really weird
purposes. We might wanna consider adding a module parameter to disable
the magic DMI table. Or do we expect them to always compile their own
kernel and know where to fix this? Thoughts?

Regards

Marcel

Re: [patch 2/2] fastboot: use a DMI match table to set defaults for port-disable

[Arjan van de Ven]

On Tue, 12 Aug 2008 03:09:11 +0200
Marcel Holtmann <marcel@holtmann.org> wrote:

> Hi Arjan,
> 
> > Subject: [PATCH] libata: use a DMI match table to set defaults for
> > port-disable
> > 
> > As suggested by Jeff:
> > This patch adds a DMI table that is used for setting defaults for
> > the new dont-probe-port parameter boot time optimization feature.
> > 
> > Initially, the table only contains the Asus EeePC 901 as this is the
> > device I have for testing.
> 
> last time I had mine open, I saw that the other ATA connectors are in
> theory usable. You have to do some heavy modifications, 

if you solder the board... well you can add a tiny kernel patch or
change the dmi ;)



-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 2/2] fastboot: use a DMI match table to set defaults for port-disable

[Alan Cox]

> if you solder the board... well you can add a tiny kernel patch or
> change the dmi ;)

Lots of people have them modified and you can even buy them pre-modded as
an end user. It needs to be simpler than hacking your kernel. In fact
this argues that the port disables should be handled by user space not by
the kernel ?

We have various other libata arguments so what would happen if instead we
pushing setting it up to userspace but fixing the libata. options line
for settings to take PCI bus info so you always hit the right port.

Yes you'd still need to fix some drivers to cope with that behaviour too
but it would kick policy (which this is) back into the hands of the
distro and end user.

Spinning a new kernel to get your off the shelf modded Eee PC wrking
isn't very user friendly.

Alan

Re: [patch 2/2] fastboot: use a DMI match table to set defaults for port-disable

[Arjan van de Ven]

On Tue, 12 Aug 2008 09:10:09 +0100
Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:

> > if you solder the board... well you can add a tiny kernel patch or
> > change the dmi ;)
> 
> Lots of people have them modified and you can even buy them
> pre-modded as an end user. It needs to be simpler than hacking your
> kernel. In fact this argues that the port disables should be handled
> by user space not by the kernel ?

which is a commandline option then; because once userspace runs you've
already paid the price in terms of boottime otherwise and would make
the idea entirely moot.


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 2/2] fastboot: use a DMI match table to set defaults for port-disable

[Alan Cox]

On Tue, 12 Aug 2008 04:37:18 -0700
Arjan van de Ven <arjan@infradead.org> wrote:

> On Tue, 12 Aug 2008 09:10:09 +0100
> Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> > > if you solder the board... well you can add a tiny kernel patch or
> > > change the dmi ;)
> > 
> > Lots of people have them modified and you can even buy them
> > pre-modded as an end user. It needs to be simpler than hacking your
> > kernel. In fact this argues that the port disables should be handled
> > by user space not by the kernel ?
> 
> which is a commandline option then; because once userspace runs you've
> already paid the price in terms of boottime otherwise and would make
> the idea entirely moot.

For the boot device yes, for the rest no. And the boot command line is
managed by the installer which is user space and vendor controlled so a
good place to put fuzzy complex logic like that.

Re: [patch 2/2] fastboot: use a DMI match table to set defaults for port-disable

[Bjorn Helgaas]

On Monday 11 August 2008 04:37:30 pm Arjan van de Ven wrote:
> 
> From: Arjan van de Ven <arjan@linux.intel.com>
> Subject: [PATCH] libata: use a DMI match table to set defaults for port-disable
> 
> As suggested by Jeff:
> This patch adds a DMI table that is used for setting defaults for
> the new dont-probe-port parameter boot time optimization feature.

Isn't this the sort of thing we theoretically could learn from ACPI?
I know we haven't really taken advantage of what ACPI tells us about
the presence of legacy devices like COM ports and IDE ports, but
maybe we should.

> +static int __init dmi_disable_ports(const struct dmi_system_id *d)
> +{
> +	int value = (unsigned long)d->driver_data;
> +
> +	printk(KERN_INFO "libata: DMI match used to disable probing\n");
> +	libata_disable_ports |= value;
> +	return 0;
> +}
> +
> +#define DMI_PORT_ATA1 (void *)0x1
> +#define DMI_PORT_ATA2 (void *)0x2
> +#define DMI_PORT_ATA3 (void *)0x4
> +#define DMI_PORT_ATA4 (void *)0x8
> +
> +static struct dmi_system_id libata_dmi_table[] __initdata = {
> +	/*
> +	 * The Asus EeePC901 has its devices attached to ATA2 only
> +	 */
> +	{
> +	.callback = dmi_disable_ports,
> +	.driver_data = DMI_PORT_ATA1,
> +	.ident = "Asus EeePC 901",
> +	.matches = {
> +		     DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK Computer INC."),
> +		     DMI_MATCH(DMI_PRODUCT_NAME, "901"),
> +		},
> +	},
> +	{}
> +};
> +
> +
>  /**
>   *	ata_force_cbl - force cable type according to libata.force
>   *	@ap: ATA port of interest
> @@ -6126,6 +6158,7 @@ static void __init ata_parse_force_param(void)
>  static int __init ata_init(void)
>  {
>  	ata_parse_force_param();
> +	dmi_check_system(libata_dmi_table);
>  
>  	ata_wq = create_workqueue("ata");
>  	if (!ata_wq)