2009-01-10 00:13:06

by Arjan van de Ven

[permalink] [raw]
Subject: libata: make parallel port scan a per-host opt-in feature

>From 17f458c7d9d7a25a04d448aef22b134135857018 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <[email protected]>
Date: Fri, 9 Jan 2009 15:54:07 -0800
Subject: [PATCH] libata: Add a per-host flag to opt-in into parallel port probes

This patch adds a per host flag that allows drivers to opt in into
having its busses scanned in parallel.

Drivers that do not set this flag get their ports scanned in
the "original" sequence.

Signed-off-by: Arjan van de Ven <[email protected]>
---
drivers/ata/ahci.c | 3 +++
drivers/ata/libata-core.c | 9 +++++++++
include/linux/libata.h | 1 +
3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 7f701cb..1fc2d4b 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -2660,6 +2660,9 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
host->iomap = pcim_iomap_table(pdev);
host->private_data = hpriv;

+ if (!hpriv->cap & HOST_CAP_SSS)
+ host->flags |= ATA_HOST_PARALLEL_SCAN;
+
if (pi.flags & ATA_FLAG_EM)
ahci_reset_em(host);

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c507a9a..31c0f22 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5920,6 +5920,15 @@ static void async_port_probe(void *data, async_cookie_t cookie)
{
int rc;
struct ata_port *ap = data;
+
+ /*
+ * If we're not allowed to scan this host in parallel,
+ * we need to wait until all previous scans have completed
+ * before going further.
+ */
+ if (!(ap->host->flags & ATA_HOST_PARALLEL_SCAN))
+ async_synchronize_cookie(cookie);
+
/* probe */
if (ap->ops->error_handler) {
struct ata_eh_info *ehi = &ap->link.eh_info;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 4f7c8fb..b6b8a7f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -239,6 +239,7 @@ enum {
/* host set flags */
ATA_HOST_SIMPLEX = (1 << 0), /* Host is simplex, one DMA channel per host only */
ATA_HOST_STARTED = (1 << 1), /* Host started */
+ ATA_HOST_PARALLEL_SCAN = (1 << 2), /* Ports on this host can be scanned in parallel */

/* bits 24:31 of host->flags are reserved for LLD specific flags */

--
1.6.0.6


--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org


2009-01-10 00:57:25

by Linus Torvalds

[permalink] [raw]
Subject: Re: libata: make parallel port scan a per-host opt-in feature



On Fri, 9 Jan 2009, Arjan van de Ven wrote:
>
> + if (!hpriv->cap & HOST_CAP_SSS)
> + host->flags |= ATA_HOST_PARALLEL_SCAN;
> +

I don't think that does what you think it does.

Linus

2009-01-10 00:58:28

by Linus Torvalds

[permalink] [raw]
Subject: Re: libata: make parallel port scan a per-host opt-in feature



On Fri, 9 Jan 2009, Linus Torvalds wrote:

>
>
> On Fri, 9 Jan 2009, Arjan van de Ven wrote:
> >
> > + if (!hpriv->cap & HOST_CAP_SSS)
> > + host->flags |= ATA_HOST_PARALLEL_SCAN;
> > +
>
> I don't think that does what you think it does.

And furthermore I'm surprised gcc doesn't warn about it. It would seem to
be an obvious thing to warn about. I'll see what a sparse warning would
look like.

Linus

2009-01-10 00:58:52

by Arjan van de Ven

[permalink] [raw]
Subject: Re: libata: make parallel port scan a per-host opt-in feature

On Fri, 9 Jan 2009 16:56:59 -0800 (PST)
Linus Torvalds <[email protected]> wrote:

>
>
> On Fri, 9 Jan 2009, Arjan van de Ven wrote:
> >
> > + if (!hpriv->cap & HOST_CAP_SSS)
> > + host->flags |= ATA_HOST_PARALLEL_SCAN;
> > +
>
> I don't think that does what you think it does.
>
> Linus
argh two few ()'s


--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2009-01-10 01:01:13

by Linus Torvalds

[permalink] [raw]
Subject: Re: libata: make parallel port scan a per-host opt-in feature



On Fri, 9 Jan 2009, Linus Torvalds wrote:
>
> On Fri, 9 Jan 2009, Linus Torvalds wrote:
> >
> > On Fri, 9 Jan 2009, Arjan van de Ven wrote:
> > >
> > > + if (!hpriv->cap & HOST_CAP_SSS)
> > > + host->flags |= ATA_HOST_PARALLEL_SCAN;
> > > +
> >
> > I don't think that does what you think it does.
>
> And furthermore I'm surprised gcc doesn't warn about it. It would seem to
> be an obvious thing to warn about. I'll see what a sparse warning would
> look like.

Heh. sparse already warns:

warning: dubious: !x & y

although I'm not sure that's necessarily the most readable warning ever.

Linus

2009-01-10 21:25:34

by Arjan van de Ven

[permalink] [raw]
Subject: [PATCH 1/2] libata: Add a per-host flag to opt-in into parallel port probes

>From 703d035d397fb2a253559f3b819ad44702807724 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <[email protected]>
Date: Fri, 9 Jan 2009 15:54:07 -0800
Subject: [PATCH] libata: Add a per-host flag to opt-in into parallel port probes

This patch adds a per host flag that allows drivers to opt in into
having its busses scanned in parallel.

Drivers that do not set this flag get their ports scanned in
the "original" sequence.

Signed-off-by: Arjan van de Ven <[email protected]>
---
drivers/ata/ahci.c | 3 +++
drivers/ata/libata-core.c | 9 +++++++++
include/linux/libata.h | 1 +
3 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 7f701cb..9603967 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -2660,6 +2660,9 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
host->iomap = pcim_iomap_table(pdev);
host->private_data = hpriv;

+ if (!(hpriv->cap & HOST_CAP_SSS))
+ host->flags |= ATA_HOST_PARALLEL_SCAN;
+
if (pi.flags & ATA_FLAG_EM)
ahci_reset_em(host);

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index c507a9a..31c0f22 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5920,6 +5920,15 @@ static void async_port_probe(void *data, async_cookie_t cookie)
{
int rc;
struct ata_port *ap = data;
+
+ /*
+ * If we're not allowed to scan this host in parallel,
+ * we need to wait until all previous scans have completed
+ * before going further.
+ */
+ if (!(ap->host->flags & ATA_HOST_PARALLEL_SCAN))
+ async_synchronize_cookie(cookie);
+
/* probe */
if (ap->ops->error_handler) {
struct ata_eh_info *ehi = &ap->link.eh_info;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 4f7c8fb..b6b8a7f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -239,6 +239,7 @@ enum {
/* host set flags */
ATA_HOST_SIMPLEX = (1 << 0), /* Host is simplex, one DMA channel per host only */
ATA_HOST_STARTED = (1 << 1), /* Host started */
+ ATA_HOST_PARALLEL_SCAN = (1 << 2), /* Ports on this host can be scanned in parallel */

/* bits 24:31 of host->flags are reserved for LLD specific flags */

--
1.6.0.6



--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2009-01-10 21:25:50

by Arjan van de Ven

[permalink] [raw]
Subject: [PATCH 2/2] libata: only ports >= 0 need to synchronize

>From 68db7c1eff38c2e121ccc922fd365f4f086369dc Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <[email protected]>
Date: Sat, 10 Jan 2009 10:18:44 -0800
Subject: [PATCH] libata: only ports >= 0 need to synchronize

In a discussio with Jeff Garzik, he mentioned that the serialization
for the libata port probes only needs to be within the domain of a host.
This means that for the first port of each host (with ID 0), we don't
need to wait, so we can relax our serialization a little.

Signed-off-by: Arjan van de Ven <[email protected]>
---
drivers/ata/libata-core.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 31c0f22..71218d7 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5925,10 +5925,12 @@ static void async_port_probe(void *data, async_cookie_t cookie)
* If we're not allowed to scan this host in parallel,
* we need to wait until all previous scans have completed
* before going further.
+ * Jeff Garzik says this is only within a controller, so we
+ * don't need to wait for port 0, only for later ports.
*/
- if (!(ap->host->flags & ATA_HOST_PARALLEL_SCAN))
+ if (!(ap->host->flags & ATA_HOST_PARALLEL_SCAN) && ap->port_no != 0)
async_synchronize_cookie(cookie);
-
+
/* probe */
if (ap->ops->error_handler) {
struct ata_eh_info *ehi = &ap->link.eh_info;
--
1.6.0.6



--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org