libata bridge limits

libata bridge limits

[Jens Axboe]

Hi,

Jeff, a long time ago we talked about the bridge limits in libata core.
Back then it was my T43 notebook, but now I'm running into the same
problem. The Mtron 70xx series SSD drives sit behind a PATA-SATA bridge
and thus runs into the bridging limits in libata. To put things into
perspective, the difference between UDMA5 and UDMA6 modes on this drive
is 85 vs 115Mb/sec or 77 vs 106Mb/sec (depends on the host controller I
tested), so it's quite a lot.

There are several ways to solve this:

a) Why was this limit put in there? It limits both transfer speed and
   request size. If it's due to some dodgy drive/bridge, perhaps we
   should just check for that and only apply the transfer limits when
   detected (or blacklisted). On the bridge setups I've seen, I've never
   had problems with killing the limit.

b) Put in a whitelist, easy to do for these Mtron drives.

c) Add a parameter to turn it on (or off, depending on the default) for
   a specific drive.

I'm in favor of a) personally, but I'd like to hear why the check was
added originally first. Dropping 20-30% of the throughput performance on
the floor without option seems like a really bad choice.

-- 
Jens Axboe

Re: libata bridge limits

[Alan Cox]

> a) Why was this limit put in there? It limits both transfer speed and
>    request size. If it's due to some dodgy drive/bridge, perhaps we
>    should just check for that and only apply the transfer limits when
>    detected (or blacklisted). On the bridge setups I've seen, I've never
>    had problems with killing the limit.

Various old bridges need it - and you can't detect the bridge type.

> 
> b) Put in a whitelist, easy to do for these Mtron drives.
> 
> c) Add a parameter to turn it on (or off, depending on the default) for
>    a specific drive.
> 
> I'm in favor of a) personally, but I'd like to hear why the check was
> added originally first. Dropping 20-30% of the throughput performance on
> the floor without option seems like a really bad choice.

Can I suggest 

d) Assume the bridge is ok but teach the SATA error handling code that if
there is a timeout immediately with such a bridge then to flip down to
UDMA5 and knobble the transfer length.

Re: libata bridge limits

[Jens Axboe]

On Tue, Aug 26 2008, Alan Cox wrote:
> > a) Why was this limit put in there? It limits both transfer speed and
> >    request size. If it's due to some dodgy drive/bridge, perhaps we
> >    should just check for that and only apply the transfer limits when
> >    detected (or blacklisted). On the bridge setups I've seen, I've never
> >    had problems with killing the limit.
> 
> Various old bridges need it - and you can't detect the bridge type.

Not generically, but for some devices (like the Mtron) we can.

> > b) Put in a whitelist, easy to do for these Mtron drives.
> > 
> > c) Add a parameter to turn it on (or off, depending on the default) for
> >    a specific drive.
> > 
> > I'm in favor of a) personally, but I'd like to hear why the check was
> > added originally first. Dropping 20-30% of the throughput performance on
> > the floor without option seems like a really bad choice.
> 
> Can I suggest 
> 
> d) Assume the bridge is ok but teach the SATA error handling code that if
> there is a timeout immediately with such a bridge then to flip down to
> UDMA5 and knobble the transfer length.

That would be nice, assuming that we can rely on safe behaviour (eg not
data corrupting badness).

-- 
Jens Axboe

Re: libata bridge limits

[Alan Cox]

> * The current IO timeouts are too long.  It's not like reducing this is
> difficult.  The only reason why we haven't reduced it yet is because we

They are too short.

> haven't been able to agree on what's the proper timeout value.
> According to Mark, 8 secs should be fine (Windows uses it) for
> read/writes but there seem to be some corner cases.

The worst case I've seen on bad blocks is up over sixty seconds and as a
result of our underlength timeouts we get continuous retries and mode
changedowns in response to this not a proper error and raid failover. The
worst case on cache flush is even longer!

We have the same problem with CD devices.

Now we should probably have a shorter timeout where we then check the
status bits for BUSY so we can quickly catch lost interrupts or commands
but that is quite different.

> * Some rare controllers fail miserably after a timeout but this is
> pretty rare and getting rarer.  I don't think we need to consider this
> the main deciding factor.

Several require resets, the driver should be doing this work. Again the
poll on timeout to check if we just lost the IRQ would improve this also
but is only done by old IDE right now.

> * Currently, the transfer speed setting reached by EH actions is not
> persistent.  On the next boot, the device would have to go through the
> same thing all over again, which isn't too pleasant.  It would be great
> if we can make this setting persistent.  Maybe this can be done libata
> sysfs and udev?

How ? you've no idea what device combination will appear next boot ?

Alan

Re: libata bridge limits

[Tejun Heo]

(cc'ing Mark Lord, Kay Sievers and Gwendal Grignou)

Jens Axboe wrote:
>> d) Assume the bridge is ok but teach the SATA error handling code that if
>> there is a timeout immediately with such a bridge then to flip down to
>> UDMA5 and knobble the transfer length.
> 
> That would be nice, assuming that we can rely on safe behaviour (eg not
> data corrupting badness).

Obstacles to such approaches are...

* The current IO timeouts are too long.  It's not like reducing this is
difficult.  The only reason why we haven't reduced it yet is because we
haven't been able to agree on what's the proper timeout value.
According to Mark, 8 secs should be fine (Windows uses it) for
read/writes but there seem to be some corner cases.

* Some rare controllers fail miserably after a timeout but this is
pretty rare and getting rarer.  I don't think we need to consider this
the main deciding factor.

* Currently, the transfer speed setting reached by EH actions is not
persistent.  On the next boot, the device would have to go through the
same thing all over again, which isn't too pleasant.  It would be great
if we can make this setting persistent.  Maybe this can be done libata
sysfs and udev?

Thanks.

-- 
tejun

Re: libata bridge limits

[Tejun Heo]

Alan Cox wrote:
>> * The current IO timeouts are too long.  It's not like reducing this is
>> difficult.  The only reason why we haven't reduced it yet is because we
> 
> They are too short.
> 
>> haven't been able to agree on what's the proper timeout value.
>> According to Mark, 8 secs should be fine (Windows uses it) for
>> read/writes but there seem to be some corner cases.
> 
> The worst case I've seen on bad blocks is up over sixty seconds and as a
> result of our underlength timeouts we get continuous retries and mode
> changedowns in response to this not a proper error and raid failover. The
> worst case on cache flush is even longer!

We probably needs to extend timeout for flush or at least when retrying
flushes.  As for reads and writes, I really think we should move more
towards shorter timeout as timeouts are not too rare in SATA at least
(and undistinguishible using BSY or other methods).  Those long delays
are very rare and maybe having control knobs is the best way to deal
with them.

> We have the same problem with CD devices.
> 
> Now we should probably have a shorter timeout where we then check the
> status bits for BUSY so we can quickly catch lost interrupts or commands
> but that is quite different.

Yeah, we need to check for lost interrupts and dead IRQ due to screaming
IRQ.  Maybe we can do some of that in interrupt core.

>> * Some rare controllers fail miserably after a timeout but this is
>> pretty rare and getting rarer.  I don't think we need to consider this
>> the main deciding factor.
> 
> Several require resets, the driver should be doing this work. Again the
> poll on timeout to check if we just lost the IRQ would improve this also
> but is only done by old IDE right now.

The few I was talking about just freezes the whole machine after a
timeout.  Dunno whether the lowlevel driver needs to do EH differently
or the controller is just built that way tho.

>> * Currently, the transfer speed setting reached by EH actions is not
>> persistent.  On the next boot, the device would have to go through the
>> same thing all over again, which isn't too pleasant.  It would be great
>> if we can make this setting persistent.  Maybe this can be done libata
>> sysfs and udev?
> 
> How ? you've no idea what device combination will appear next boot ?

udev and hal know a lot about the system configuration including
connection topology and many ways to id each device.  Saving limit
configuration using combination of topology and device ID should be
pretty safe.  I think more difficult problem is how to notify the user
that such persistent auto-configuration happened and provide a
convenient way to undo.

Thanks.

-- 
tejun

Re: libata bridge limits

[Alan Cox]

> > Now we should probably have a shorter timeout where we then check the
> > status bits for BUSY so we can quickly catch lost interrupts or commands
> > but that is quite different.
> 
> Yeah, we need to check for lost interrupts and dead IRQ due to screaming
> IRQ.  Maybe we can do some of that in interrupt core.

For SFF at least we can read altstatus and check for BUSY. If BUSY is not
set then something is up, either we have an error we didn't get an IRQ
for or the status is successful.

> The few I was talking about just freezes the whole machine after a
> timeout.  Dunno whether the lowlevel driver needs to do EH differently
> or the controller is just built that way tho.

Some lock the box solid if you don't reset the controller before you
touch any registers on a timeout. I thought we had them all covered - do
you know which controllers are still showing this ?

Alan

Re: libata bridge limits

[Brad Campbell]

Jens 2 wrote:
> On Tue, Aug 26 2008, Alan Cox wrote:
>>> a) Why was this limit put in there? It limits both transfer speed and
>>>    request size. If it's due to some dodgy drive/bridge, perhaps we
>>>    should just check for that and only apply the transfer limits when
>>>    detected (or blacklisted). On the bridge setups I've seen, I've never
>>>    had problems with killing the limit.
>> Various old bridges need it - and you can't detect the bridge type.
> 
> Not generically, but for some devices (like the Mtron) we can.
> 
>>> b) Put in a whitelist, easy to do for these Mtron drives.
>>>
>>> c) Add a parameter to turn it on (or off, depending on the default) for
>>>    a specific drive.
>>>
>>> I'm in favor of a) personally, but I'd like to hear why the check was
>>> added originally first. Dropping 20-30% of the throughput performance on
>>> the floor without option seems like a really bad choice.

The check was originally put there as some nasty bridges caused all sorts of errors when these 
limits were exceeded, including some random data corruption from memory.

Those particular bridges were/are still widely available an can't be detected / identified using any 
other means.

>> Can I suggest 
>>
>> d) Assume the bridge is ok but teach the SATA error handling code that if
>> there is a timeout immediately with such a bridge then to flip down to
>> UDMA5 and knobble the transfer length.
> 
> That would be nice, assuming that we can rely on safe behaviour (eg not
> data corrupting badness).
> 

I was responsible for that original bridge knobble stuff and fortunately I still have the bridges, 
disks and controllers that triggered the original faults. If someone wants to cook up some code for 
testing I'd be more than happy to stick this stuff on the bench and beat on it for regression purposes.

Brad
-- 
Dolphins are so intelligent that within a few weeks they can
train Americans to stand at the edge of the pool and throw them
fish.

Re: libata bridge limits

[Tejun Heo]

Alan Cox wrote:
>>> Now we should probably have a shorter timeout where we then check the
>>> status bits for BUSY so we can quickly catch lost interrupts or commands
>>> but that is quite different.
>> Yeah, we need to check for lost interrupts and dead IRQ due to screaming
>> IRQ.  Maybe we can do some of that in interrupt core.
> 
> For SFF at least we can read altstatus and check for BUSY. If BUSY is not
> set then something is up, either we have an error we didn't get an IRQ
> for or the status is successful.
> 
>> The few I was talking about just freezes the whole machine after a
>> timeout.  Dunno whether the lowlevel driver needs to do EH differently
>> or the controller is just built that way tho.
> 
> Some lock the box solid if you don't reset the controller before you
> touch any registers on a timeout. I thought we had them all covered - do
> you know which controllers are still showing this ?

IIRC some very early via SATAs lock up the machine solid no matter what
you do unless a command is completed by the device side.  Maybe it
requires special sequence to abort in-flight commands but simple SRST
wasn't enough.  :-(

-- 
tejun

Re: libata bridge limits

[Jens Axboe]

On Tue, Aug 26 2008, Brad Campbell wrote:
> Jens 2 wrote:
> >On Tue, Aug 26 2008, Alan Cox wrote:
> >>>a) Why was this limit put in there? It limits both transfer speed and
> >>>   request size. If it's due to some dodgy drive/bridge, perhaps we
> >>>   should just check for that and only apply the transfer limits when
> >>>   detected (or blacklisted). On the bridge setups I've seen, I've never
> >>>   had problems with killing the limit.
> >>Various old bridges need it - and you can't detect the bridge type.
> >
> >Not generically, but for some devices (like the Mtron) we can.
> >
> >>>b) Put in a whitelist, easy to do for these Mtron drives.
> >>>
> >>>c) Add a parameter to turn it on (or off, depending on the default) for
> >>>   a specific drive.
> >>>
> >>>I'm in favor of a) personally, but I'd like to hear why the check was
> >>>added originally first. Dropping 20-30% of the throughput performance on
> >>>the floor without option seems like a really bad choice.
> 
> The check was originally put there as some nasty bridges caused all sorts 
> of errors when these limits were exceeded, including some random data 
> corruption from memory.
> 
> Those particular bridges were/are still widely available an can't be 
> detected / identified using any other means.

That's a worry, since I don't think we can dynamically switch back in
that case if it has potential data corruption problems.

> >>Can I suggest 
> >>
> >>d) Assume the bridge is ok but teach the SATA error handling code that if
> >>there is a timeout immediately with such a bridge then to flip down to
> >>UDMA5 and knobble the transfer length.
> >
> >That would be nice, assuming that we can rely on safe behaviour (eg not
> >data corrupting badness).
> >
> 
> I was responsible for that original bridge knobble stuff and fortunately I 
> still have the bridges, disks and controllers that triggered the original 
> faults. If someone wants to cook up some code for testing I'd be more than 
> happy to stick this stuff on the bench and beat on it for regression 
> purposes.

Given that this problem should be going away and that it only really
matters on very select devices (like this SSD), I think we should just
add a quick white list for the bridge limits.

Below is a quick'n dirty for that...

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 79e3a8e..fe8033a 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2097,9 +2097,70 @@ retry:
 	return rc;
 }
 
+struct ata_blacklist_entry {
+	const char *model_num;
+	const char *model_rev;
+	unsigned long horkage;
+};
+
+static const struct ata_blacklist_entry ata_bridge_whitelist[] = {
+	/*
+	 * The following devices sit behind a bridge, but don't need
+	 * transfer rate or size limits applied.
+	 */
+	{ "Mtron", },
+
+	/* End Marker */
+	{ }
+};
+
+static int strn_pattern_cmp(const char *patt, const char *name, int wildchar)
+{
+	const char *p;
+	int len;
+
+	/*
+	 * check for trailing wildcard: *\0
+	 */
+	p = strchr(patt, wildchar);
+	if (p && ((*(p + 1)) == 0))
+		len = p - patt;
+	else {
+		len = strlen(name);
+		if (!len) {
+			if (!*patt)
+				return 0;
+			return -1;
+		}
+	}
+
+	return strncmp(patt, name, len);
+}
+
+static unsigned int ata_dev_bridge_whitelisted(const struct ata_device *dev)
+{
+	unsigned char model_num[ATA_ID_PROD_LEN + 1];
+	unsigned char model_rev[ATA_ID_FW_REV_LEN + 1];
+	const struct ata_blacklist_entry *ad = ata_bridge_whitelist;
+
+	ata_id_c_string(dev->id, model_num, ATA_ID_PROD, sizeof(model_num));
+	ata_id_c_string(dev->id, model_rev, ATA_ID_FW_REV, sizeof(model_rev));
+
+	while (ad->model_num) {
+		if (!strn_pattern_cmp(ad->model_num, model_num, '*'))
+			return 1;
+		ad++;
+	}
+	return 0;
+}
+
 static inline u8 ata_dev_knobble(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
+
+	if (ata_dev_bridge_whitelisted(dev))
+		return 0;
+
 	return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
 }
 
@@ -3913,12 +3974,6 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
 	return rc;
 }
 
-struct ata_blacklist_entry {
-	const char *model_num;
-	const char *model_rev;
-	unsigned long horkage;
-};
-
 static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	/* Devices with DMA related problems under Linux */
 	{ "WDC AC11000H",	NULL,		ATA_HORKAGE_NODMA },
@@ -4002,29 +4057,6 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	{ }
 };
 
-static int strn_pattern_cmp(const char *patt, const char *name, int wildchar)
-{
-	const char *p;
-	int len;
-
-	/*
-	 * check for trailing wildcard: *\0
-	 */
-	p = strchr(patt, wildchar);
-	if (p && ((*(p + 1)) == 0))
-		len = p - patt;
-	else {
-		len = strlen(name);
-		if (!len) {
-			if (!*patt)
-				return 0;
-			return -1;
-		}
-	}
-
-	return strncmp(patt, name, len);
-}
-
 static unsigned long ata_dev_blacklisted(const struct ata_device *dev)
 {
 	unsigned char model_num[ATA_ID_PROD_LEN + 1];

-- 
Jens Axboe

Re: libata bridge limits

[Tejun Heo]

Jens Axboe wrote:
> Given that this problem should be going away and that it only really
> matters on very select devices (like this SSD), I think we should just
> add a quick white list for the bridge limits.

Yeah, it sucks that up & coming SSDs are still using PATA-SATA bridges.
 The expectation when adding the wildcard limitation was that those P/S
bridges are not gonna be around for too long and the limit is most
likely not be an actual problem.  Oh well...

> Below is a quick'n dirty for that...
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 79e3a8e..fe8033a 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2097,9 +2097,70 @@ retry:
>  	return rc;
>  }
>  
> +struct ata_blacklist_entry {
> +	const char *model_num;
> +	const char *model_rev;
> +	unsigned long horkage;
> +};
> +
> +static const struct ata_blacklist_entry ata_bridge_whitelist[] = {
> +	/*
> +	 * The following devices sit behind a bridge, but don't need
> +	 * transfer rate or size limits applied.
> +	 */
> +	{ "Mtron", },
> +
> +	/* End Marker */
> +	{ }
> +};

Any reason this can't be part of the existing blacklist?  It already
supports wildcard matching and all.

Thanks.

-- 
tejun

Re: libata bridge limits

[Jens Axboe]

On Tue, Aug 26 2008, Tejun Heo wrote:
> Jens Axboe wrote:
> > Given that this problem should be going away and that it only really
> > matters on very select devices (like this SSD), I think we should just
> > add a quick white list for the bridge limits.
> 
> Yeah, it sucks that up & coming SSDs are still using PATA-SATA bridges.
>  The expectation when adding the wildcard limitation was that those P/S
> bridges are not gonna be around for too long and the limit is most
> likely not be an actual problem.  Oh well...

I would hope that only the current generation do that, and to be honest
I'm still pretty baffled that they exist :-)

> > Below is a quick'n dirty for that...
> > 
> > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> > index 79e3a8e..fe8033a 100644
> > --- a/drivers/ata/libata-core.c
> > +++ b/drivers/ata/libata-core.c
> > @@ -2097,9 +2097,70 @@ retry:
> >  	return rc;
> >  }
> >  
> > +struct ata_blacklist_entry {
> > +	const char *model_num;
> > +	const char *model_rev;
> > +	unsigned long horkage;
> > +};
> > +
> > +static const struct ata_blacklist_entry ata_bridge_whitelist[] = {
> > +	/*
> > +	 * The following devices sit behind a bridge, but don't need
> > +	 * transfer rate or size limits applied.
> > +	 */
> > +	{ "Mtron", },
> > +
> > +	/* End Marker */
> > +	{ }
> > +};
> 
> Any reason this can't be part of the existing blacklist?  It already
> supports wildcard matching and all.

Sure, just with an inverse flag, I'll do that.

-- 
Jens Axboe

Re: libata bridge limits

[Jens Axboe]

On Tue, Aug 26 2008, Jens Axboe wrote:
> On Tue, Aug 26 2008, Tejun Heo wrote:
> > Jens Axboe wrote:
> > > Given that this problem should be going away and that it only really
> > > matters on very select devices (like this SSD), I think we should just
> > > add a quick white list for the bridge limits.
> > 
> > Yeah, it sucks that up & coming SSDs are still using PATA-SATA bridges.
> >  The expectation when adding the wildcard limitation was that those P/S
> > bridges are not gonna be around for too long and the limit is most
> > likely not be an actual problem.  Oh well...
> 
> I would hope that only the current generation do that, and to be honest
> I'm still pretty baffled that they exist :-)
> 
> > > Below is a quick'n dirty for that...
> > > 
> > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> > > index 79e3a8e..fe8033a 100644
> > > --- a/drivers/ata/libata-core.c
> > > +++ b/drivers/ata/libata-core.c
> > > @@ -2097,9 +2097,70 @@ retry:
> > >  	return rc;
> > >  }
> > >  
> > > +struct ata_blacklist_entry {
> > > +	const char *model_num;
> > > +	const char *model_rev;
> > > +	unsigned long horkage;
> > > +};
> > > +
> > > +static const struct ata_blacklist_entry ata_bridge_whitelist[] = {
> > > +	/*
> > > +	 * The following devices sit behind a bridge, but don't need
> > > +	 * transfer rate or size limits applied.
> > > +	 */
> > > +	{ "Mtron", },
> > > +
> > > +	/* End Marker */
> > > +	{ }
> > > +};
> > 
> > Any reason this can't be part of the existing blacklist?  It already
> > supports wildcard matching and all.
> 
> Sure, just with an inverse flag, I'll do that.

OK, something like this. Jeff, if you think this is fine, let me know
and I'll submit a proper patch with description and so on.

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 79e3a8e..879ceac 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2100,6 +2100,10 @@ retry:
 static inline u8 ata_dev_knobble(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
+
+	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_BRIDGE_OK)
+		return 0;
+
 	return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
 }
 
@@ -3998,6 +4002,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	{ "TSSTcorp CDDVDW SH-S202N", "SB00",	  ATA_HORKAGE_IVB, },
 	{ "TSSTcorp CDDVDW SH-S202N", "SB01",	  ATA_HORKAGE_IVB, },
 
+	/* Devices that do not need bridging limits applied */
+	{ "Mtron",		NULL,		0, },
+
 	/* End Marker */
 	{ }
 };
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 225bfc5..9f194c0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -364,6 +364,7 @@ enum {
 	ATA_HORKAGE_IPM		= (1 << 7),	/* Link PM problems */
 	ATA_HORKAGE_IVB		= (1 << 8),	/* cbl det validity bit bugs */
 	ATA_HORKAGE_STUCK_ERR	= (1 << 9),	/* stuck ERR on next PACKET */
+	ATA_HORKAGE_BRIDGE_OK	= (1 << 10),	/* no bridge limits */
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */

-- 
Jens Axboe

Re: libata bridge limits

[Tejun Heo]

Jens Axboe wrote:
> OK, something like this. Jeff, if you think this is fine, let me know
> and I'll submit a proper patch with description and so on.

FWIW, looks good to me.  Just one small concern below...

> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 79e3a8e..879ceac 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2100,6 +2100,10 @@ retry:
>  static inline u8 ata_dev_knobble(struct ata_device *dev)
>  {
>  	struct ata_port *ap = dev->link->ap;
> +
> +	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_BRIDGE_OK)
> +		return 0;
> +
>  	return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
>  }
>  
> @@ -3998,6 +4002,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
>  	{ "TSSTcorp CDDVDW SH-S202N", "SB00",	  ATA_HORKAGE_IVB, },
>  	{ "TSSTcorp CDDVDW SH-S202N", "SB01",	  ATA_HORKAGE_IVB, },
>  
> +	/* Devices that do not need bridging limits applied */
> +	{ "Mtron",		NULL,		0, },

"Mtron" looks like a broad match but then again for some reason many
non-traditional ATA vendors don't like to give descriptive
identifications to their devices.  Any chance there is something more to
match?  :-(

Thanks.

-- 
tejun

Re: libata bridge limits

[Jens Axboe]

On Tue, Aug 26 2008, Jens Axboe wrote:
> On Tue, Aug 26 2008, Jens Axboe wrote:
> > On Tue, Aug 26 2008, Tejun Heo wrote:
> > > Jens Axboe wrote:
> > > > Given that this problem should be going away and that it only really
> > > > matters on very select devices (like this SSD), I think we should just
> > > > add a quick white list for the bridge limits.
> > > 
> > > Yeah, it sucks that up & coming SSDs are still using PATA-SATA bridges.
> > >  The expectation when adding the wildcard limitation was that those P/S
> > > bridges are not gonna be around for too long and the limit is most
> > > likely not be an actual problem.  Oh well...
> > 
> > I would hope that only the current generation do that, and to be honest
> > I'm still pretty baffled that they exist :-)
> > 
> > > > Below is a quick'n dirty for that...
> > > > 
> > > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> > > > index 79e3a8e..fe8033a 100644
> > > > --- a/drivers/ata/libata-core.c
> > > > +++ b/drivers/ata/libata-core.c
> > > > @@ -2097,9 +2097,70 @@ retry:
> > > >  	return rc;
> > > >  }
> > > >  
> > > > +struct ata_blacklist_entry {
> > > > +	const char *model_num;
> > > > +	const char *model_rev;
> > > > +	unsigned long horkage;
> > > > +};
> > > > +
> > > > +static const struct ata_blacklist_entry ata_bridge_whitelist[] = {
> > > > +	/*
> > > > +	 * The following devices sit behind a bridge, but don't need
> > > > +	 * transfer rate or size limits applied.
> > > > +	 */
> > > > +	{ "Mtron", },
> > > > +
> > > > +	/* End Marker */
> > > > +	{ }
> > > > +};
> > > 
> > > Any reason this can't be part of the existing blacklist?  It already
> > > supports wildcard matching and all.
> > 
> > Sure, just with an inverse flag, I'll do that.
> 
> OK, something like this. Jeff, if you think this is fine, let me know
> and I'll submit a proper patch with description and so on.

This one has a better chance of actually working...

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 79e3a8e..f3bda2c 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2100,6 +2100,10 @@ retry:
 static inline u8 ata_dev_knobble(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
+
+	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_BRIDGE_OK)
+		return 0;
+
 	return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
 }
 
@@ -3998,6 +4002,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	{ "TSSTcorp CDDVDW SH-S202N", "SB00",	  ATA_HORKAGE_IVB, },
 	{ "TSSTcorp CDDVDW SH-S202N", "SB01",	  ATA_HORKAGE_IVB, },
 
+	/* Devices that do not need bridging limits applied */
+	{ "MTRON",		NULL,		ATA_HORKAGE_BRIDGE_OK, },
+
 	/* End Marker */
 	{ }
 };
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 225bfc5..9f194c0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -364,6 +364,7 @@ enum {
 	ATA_HORKAGE_IPM		= (1 << 7),	/* Link PM problems */
 	ATA_HORKAGE_IVB		= (1 << 8),	/* cbl det validity bit bugs */
 	ATA_HORKAGE_STUCK_ERR	= (1 << 9),	/* stuck ERR on next PACKET */
+	ATA_HORKAGE_BRIDGE_OK	= (1 << 10),	/* no bridge limits */
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */

-- 
Jens Axboe

Re: libata bridge limits

[Jens Axboe]

On Tue, Aug 26 2008, Tejun Heo wrote:
> Jens Axboe wrote:
> > OK, something like this. Jeff, if you think this is fine, let me know
> > and I'll submit a proper patch with description and so on.
> 
> FWIW, looks good to me.  Just one small concern below...
> 
> > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> > index 79e3a8e..879ceac 100644
> > --- a/drivers/ata/libata-core.c
> > +++ b/drivers/ata/libata-core.c
> > @@ -2100,6 +2100,10 @@ retry:
> >  static inline u8 ata_dev_knobble(struct ata_device *dev)
> >  {
> >  	struct ata_port *ap = dev->link->ap;
> > +
> > +	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_BRIDGE_OK)
> > +		return 0;
> > +
> >  	return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
> >  }
> >  
> > @@ -3998,6 +4002,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
> >  	{ "TSSTcorp CDDVDW SH-S202N", "SB00",	  ATA_HORKAGE_IVB, },
> >  	{ "TSSTcorp CDDVDW SH-S202N", "SB01",	  ATA_HORKAGE_IVB, },
> >  
> > +	/* Devices that do not need bridging limits applied */
> > +	{ "Mtron",		NULL,		0, },
> 
> "Mtron" looks like a broad match but then again for some reason many
> non-traditional ATA vendors don't like to give descriptive
> identifications to their devices.  Any chance there is something more to
> match?  :-(

We could make it "MTRON MSP", I think that covers both the 70xx and 75xx
range.

-- 
Jens Axboe

Re: libata bridge limits

[Gwendal Grignou]

The current patch  to implement libata-sysfs floating in the mailing
list is read-only.
For write we must currently go through SCSI midlayer ioctls and use
sat. To send commands directly within libata[-sysfs] - for instance
writing SControl register of a SATA PM - I need to either implement a
queue mechanism over ata_exec_internal() or schedule eh and piggy back
on it.

Once implemented, we could definitely use it in conjunction with udev
to tune ATA devices.

Gwendal.

On Tue, Aug 26, 2008 at 3:43 AM, Tejun Heo <tj@kernel.org> wrote:
> (cc'ing Mark Lord, Kay Sievers and Gwendal Grignou)
>
> Jens Axboe wrote:
>>> d) Assume the bridge is ok but teach the SATA error handling code that if
>>> there is a timeout immediately with such a bridge then to flip down to
>>> UDMA5 and knobble the transfer length.
>>
>> That would be nice, assuming that we can rely on safe behaviour (eg not
>> data corrupting badness).
>
> Obstacles to such approaches are...
>
> * The current IO timeouts are too long.  It's not like reducing this is
> difficult.  The only reason why we haven't reduced it yet is because we
> haven't been able to agree on what's the proper timeout value.
> According to Mark, 8 secs should be fine (Windows uses it) for
> read/writes but there seem to be some corner cases.
>
> * Some rare controllers fail miserably after a timeout but this is
> pretty rare and getting rarer.  I don't think we need to consider this
> the main deciding factor.
>
> * Currently, the transfer speed setting reached by EH actions is not
> persistent.  On the next boot, the device would have to go through the
> same thing all over again, which isn't too pleasant.  It would be great
> if we can make this setting persistent.  Maybe this can be done libata
> sysfs and udev?
>
> Thanks.
>
> --
> tejun
>

Re: libata bridge limits

[James Bottomley]

On Tue, 2008-08-26 at 10:25 -0700, Gwendal Grignou wrote:
> The current patch  to implement libata-sysfs floating in the mailing
> list is read-only.
> For write we must currently go through SCSI midlayer ioctls and use
> sat. To send commands directly within libata[-sysfs] - for instance
> writing SControl register of a SATA PM - I need to either implement a
> queue mechanism over ata_exec_internal() or schedule eh and piggy back
> on it.

That's not how the other transport classes work, why does the libata one
need to go through the mid-layer (well, except for the necessary command
issue stuff, of course).  The design of the transport classes was
specifically to take all transport knowledge out of the mid layer and
connect the requisite controls in sysfs directly to the transport
actions.

The (semi)direct analogue of writing SControl of a SATA PM is writing a
phy control parameter of a SAS expander ... that's all accomplished in
libsas/scsi_transport_sata without ever going via the mid-layer.

> Once implemented, we could definitely use it in conjunction with udev
> to tune ATA devices.

James

Re: libata bridge limits

[Gwendal Grignou]

On Tue, Aug 26, 2008 at 10:45 AM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Tue, 2008-08-26 at 10:25 -0700, Gwendal Grignou wrote:
>> The current patch  to implement libata-sysfs floating in the mailing
>> list is read-only.
>> For write we must currently go through SCSI midlayer ioctls and use
>> sat. To send commands directly within libata[-sysfs] - for instance
>> writing SControl register of a SATA PM - I need to either implement a
>> queue mechanism over ata_exec_internal() or schedule eh and piggy back
>> on it.
>
> That's not how the other transport classes work, why does the libata one
> need to go through the mid-layer (well, except for the necessary command
> issue stuff, of course).  The design of the transport classes was
> specifically to take all transport knowledge out of the mid layer and
> connect the requisite controls in sysfs directly to the transport
> actions.
I agree. I don't want to use the mid-layer either. I just mentioned
that if I want to send a raw taskfile to a ATA device today, the only
way I found is using ata_task_ioctl() which relies on the mid-layer to
schedule the request; the direct way - through ata_exec_internal() -
assumes only one outstanding command at a time.  I need to implement
something else for the accessing ATA devices in libata-sysfs similar
to smp_execute_task() in libsas.

>
> The (semi)direct analogue of writing SControl of a SATA PM is writing a
> phy control parameter of a SAS expander ... that's all accomplished in
> libsas/scsi_transport_sata without ever going via the mid-layer.
Agree.

Gwendal.
>
>> Once implemented, we could definitely use it in conjunction with udev
>> to tune ATA devices.
>
> James
>
>
>

Re: libata bridge limits

[Jeff Garzik]

Jens Axboe wrote:
> OK, something like this. Jeff, if you think this is fine, let me know
> and I'll submit a proper patch with description and so on.
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 79e3a8e..879ceac 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2100,6 +2100,10 @@ retry:
>  static inline u8 ata_dev_knobble(struct ata_device *dev)
>  {
>  	struct ata_port *ap = dev->link->ap;
> +
> +	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_BRIDGE_OK)
> +		return 0;
> +
>  	return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
>  }
>  
> @@ -3998,6 +4002,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
>  	{ "TSSTcorp CDDVDW SH-S202N", "SB00",	  ATA_HORKAGE_IVB, },
>  	{ "TSSTcorp CDDVDW SH-S202N", "SB01",	  ATA_HORKAGE_IVB, },
>  
> +	/* Devices that do not need bridging limits applied */
> +	{ "Mtron",		NULL,		0, },
> +
>  	/* End Marker */
>  	{ }
>  };
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 225bfc5..9f194c0 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -364,6 +364,7 @@ enum {
>  	ATA_HORKAGE_IPM		= (1 << 7),	/* Link PM problems */
>  	ATA_HORKAGE_IVB		= (1 << 8),	/* cbl det validity bit bugs */
>  	ATA_HORKAGE_STUCK_ERR	= (1 << 9),	/* stuck ERR on next PACKET */
> +	ATA_HORKAGE_BRIDGE_OK	= (1 << 10),	/* no bridge limits */
>  
>  	 /* DMA mask for user DMA control: User visible values; DO NOT
>  	    renumber */


This one seems fine to me...

	Jeff

Re: libata bridge limits

[James Bottomley]

On Tue, 2008-08-26 at 12:25 -0700, Gwendal Grignou wrote:
> On Tue, Aug 26, 2008 at 10:45 AM, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > On Tue, 2008-08-26 at 10:25 -0700, Gwendal Grignou wrote:
> >> The current patch  to implement libata-sysfs floating in the mailing
> >> list is read-only.
> >> For write we must currently go through SCSI midlayer ioctls and use
> >> sat. To send commands directly within libata[-sysfs] - for instance
> >> writing SControl register of a SATA PM - I need to either implement a
> >> queue mechanism over ata_exec_internal() or schedule eh and piggy back
> >> on it.
> >
> > That's not how the other transport classes work, why does the libata one
> > need to go through the mid-layer (well, except for the necessary command
> > issue stuff, of course).  The design of the transport classes was
> > specifically to take all transport knowledge out of the mid layer and
> > connect the requisite controls in sysfs directly to the transport
> > actions.
> I agree. I don't want to use the mid-layer either. I just mentioned
> that if I want to send a raw taskfile to a ATA device today, the only
> way I found is using ata_task_ioctl() which relies on the mid-layer to
> schedule the request; the direct way - through ata_exec_internal() -
> assumes only one outstanding command at a time.  I need to implement
> something else for the accessing ATA devices in libata-sysfs similar
> to smp_execute_task() in libsas.

Actually, no ... if you're sending a SATA task, then you're right, you
need to use the mid-layer inputs.  scsi_execute() is the way transport
classes use this (see scsi_transport_spi ... it does exactly this for
domain validation).  ata_task_ioctl() does it exactly this way.

You don't want something like smp_execute_task because that's an
internal libsas API for doing out of band management tasks ... ATA has
no concept of anything like this.  Even our publicly exposed frame
in/frame out interfaces for performing SMP tasks go via a special API
stream in the transport class framework so they can be issued on all SAS
hosts.  Mechanistically, they're different from normal SCSI commands
because they don't count against the queuing quota or have ordinary
tags.  If your commands come out again as standard ATA commands, I 
suspect they have to go through the standard ATA quota machinery.

SAS is very different from SATA in that we have tons of addressable
ports sitting off the devices, each addressable by predefined protocols,
so what smp_execute_task is doing is addressing the SMP port ... this is
completely out of band as far as the normal SSP/STP protocols see
things.  With port multipliers, you're more or less stuck with an
in-band type of packet.

James

Re: libata bridge limits

[Mark Lord]

Jeff Garzik wrote:
> Jens Axboe wrote:
>> OK, something like this. Jeff, if you think this is fine, let me know
>> and I'll submit a proper patch with description and so on.
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index 79e3a8e..879ceac 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -2100,6 +2100,10 @@ retry:
>>  static inline u8 ata_dev_knobble(struct ata_device *dev)
>>  {
>>      struct ata_port *ap = dev->link->ap;
>> +
>> +    if (ata_dev_blacklisted(dev) & ATA_HORKAGE_BRIDGE_OK)
>> +        return 0;
>> +
>>      return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
>>  }
>>  
>> @@ -3998,6 +4002,9 @@ static const struct ata_blacklist_entry 
>> ata_device_blacklist [] = {
>>      { "TSSTcorp CDDVDW SH-S202N", "SB00",      ATA_HORKAGE_IVB, },
>>      { "TSSTcorp CDDVDW SH-S202N", "SB01",      ATA_HORKAGE_IVB, },
>>  
>> +    /* Devices that do not need bridging limits applied */
>> +    { "Mtron",        NULL,        0, },

Err.. shouldn't there be an ATA_HORKAGE_BRIDGE_OK on that line?
Or did Jens already repost a corrected patch..

>> +
>>      /* End Marker */
>>      { }
>>  };
>> diff --git a/include/linux/libata.h b/include/linux/libata.h
>> index 225bfc5..9f194c0 100644
>> --- a/include/linux/libata.h
>> +++ b/include/linux/libata.h
>> @@ -364,6 +364,7 @@ enum {
>>      ATA_HORKAGE_IPM        = (1 << 7),    /* Link PM problems */
>>      ATA_HORKAGE_IVB        = (1 << 8),    /* cbl det validity bit 
>> bugs */
>>      ATA_HORKAGE_STUCK_ERR    = (1 << 9),    /* stuck ERR on next 
>> PACKET */
>> +    ATA_HORKAGE_BRIDGE_OK    = (1 << 10),    /* no bridge limits */
>>  
>>       /* DMA mask for user DMA control: User visible values; DO NOT
>>          renumber */
> 
> 
> This one seems fine to me...
..

Re: libata bridge limits

[Jens Axboe]

On Tue, Aug 26 2008, Jeff Garzik wrote:
> Jens Axboe wrote:
> >OK, something like this. Jeff, if you think this is fine, let me know
> >and I'll submit a proper patch with description and so on.
> >
> >diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> >index 79e3a8e..879ceac 100644
> >--- a/drivers/ata/libata-core.c
> >+++ b/drivers/ata/libata-core.c
> >@@ -2100,6 +2100,10 @@ retry:
> > static inline u8 ata_dev_knobble(struct ata_device *dev)
> > {
> > 	struct ata_port *ap = dev->link->ap;
> >+
> >+	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_BRIDGE_OK)
> >+		return 0;
> >+
> > 	return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
> > }
> > 
> >@@ -3998,6 +4002,9 @@ static const struct ata_blacklist_entry 
> >ata_device_blacklist [] = {
> > 	{ "TSSTcorp CDDVDW SH-S202N", "SB00",	  ATA_HORKAGE_IVB, },
> > 	{ "TSSTcorp CDDVDW SH-S202N", "SB01",	  ATA_HORKAGE_IVB, },
> > 
> >+	/* Devices that do not need bridging limits applied */
> >+	{ "Mtron",		NULL,		0, },
> >+
> > 	/* End Marker */
> > 	{ }
> > };
> >diff --git a/include/linux/libata.h b/include/linux/libata.h
> >index 225bfc5..9f194c0 100644
> >--- a/include/linux/libata.h
> >+++ b/include/linux/libata.h
> >@@ -364,6 +364,7 @@ enum {
> > 	ATA_HORKAGE_IPM		= (1 << 7),	/* Link PM problems */
> > 	ATA_HORKAGE_IVB		= (1 << 8),	/* cbl det validity bit bugs 
> > 	*/
> > 	ATA_HORKAGE_STUCK_ERR	= (1 << 9),	/* stuck ERR on next PACKET 
> > 	*/
> >+	ATA_HORKAGE_BRIDGE_OK	= (1 << 10),	/* no bridge limits */
> > 
> > 	 /* DMA mask for user DMA control: User visible values; DO NOT
> > 	    renumber */
> 
> 
> This one seems fine to me...

OK, so here's a version that I also tested. I applied the bridge ok flag
to MTRON MSP-SATA, that is the closest match to the 70xx and 75xx
series.

---

From: Jens Axboe <jens.axboe@oracle.com>
Subject: [PATCH] libata: add whitelist for devices with known good pata-sata bridges

libata currently imposes a UDMA5 max transfer rate and 200 sector max
transfer size for SATA devices that sit behind a pata-sata bridge. Lots
of devices have known good bridges that don't need this limit applied.
The MTRON SSD disks are such devices. Transfer rates are increased by
20-30% with the restriction removed.

So add a "blacklist" entry for the MTRON devices, with a flag indicating
that the bridge is known good.

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 79e3a8e..78cd9b3 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2100,6 +2100,10 @@ retry:
 static inline u8 ata_dev_knobble(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
+
+	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_BRIDGE_OK)
+		return 0;
+
 	return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
 }
 
@@ -3998,6 +4002,9 @@ static const struct ata_blacklist_entry ata_device_blacklist [] = {
 	{ "TSSTcorp CDDVDW SH-S202N", "SB00",	  ATA_HORKAGE_IVB, },
 	{ "TSSTcorp CDDVDW SH-S202N", "SB01",	  ATA_HORKAGE_IVB, },
 
+	/* Devices that do not need bridging limits applied */
+	{ "MTRON MSP-SATA*",		NULL,	ATA_HORKAGE_BRIDGE_OK, },
+
 	/* End Marker */
 	{ }
 };
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 225bfc5..9f194c0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -364,6 +364,7 @@ enum {
 	ATA_HORKAGE_IPM		= (1 << 7),	/* Link PM problems */
 	ATA_HORKAGE_IVB		= (1 << 8),	/* cbl det validity bit bugs */
 	ATA_HORKAGE_STUCK_ERR	= (1 << 9),	/* stuck ERR on next PACKET */
+	ATA_HORKAGE_BRIDGE_OK	= (1 << 10),	/* no bridge limits */
 
 	 /* DMA mask for user DMA control: User visible values; DO NOT
 	    renumber */

-- 
Jens Axboe

Re: libata bridge limits

[Jeff Garzik]

Jens Axboe wrote:
> On Tue, Aug 26 2008, Jeff Garzik wrote:
>> Jens Axboe wrote:
>>> OK, something like this. Jeff, if you think this is fine, let me know
>>> and I'll submit a proper patch with description and so on.
>>>
>>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>>> index 79e3a8e..879ceac 100644
>>> --- a/drivers/ata/libata-core.c
>>> +++ b/drivers/ata/libata-core.c
>>> @@ -2100,6 +2100,10 @@ retry:
>>> static inline u8 ata_dev_knobble(struct ata_device *dev)
>>> {
>>> 	struct ata_port *ap = dev->link->ap;
>>> +
>>> +	if (ata_dev_blacklisted(dev) & ATA_HORKAGE_BRIDGE_OK)
>>> +		return 0;
>>> +
>>> 	return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
>>> }
>>>
>>> @@ -3998,6 +4002,9 @@ static const struct ata_blacklist_entry 
>>> ata_device_blacklist [] = {
>>> 	{ "TSSTcorp CDDVDW SH-S202N", "SB00",	  ATA_HORKAGE_IVB, },
>>> 	{ "TSSTcorp CDDVDW SH-S202N", "SB01",	  ATA_HORKAGE_IVB, },
>>>
>>> +	/* Devices that do not need bridging limits applied */
>>> +	{ "Mtron",		NULL,		0, },
>>> +
>>> 	/* End Marker */
>>> 	{ }
>>> };
>>> diff --git a/include/linux/libata.h b/include/linux/libata.h
>>> index 225bfc5..9f194c0 100644
>>> --- a/include/linux/libata.h
>>> +++ b/include/linux/libata.h
>>> @@ -364,6 +364,7 @@ enum {
>>> 	ATA_HORKAGE_IPM		= (1 << 7),	/* Link PM problems */
>>> 	ATA_HORKAGE_IVB		= (1 << 8),	/* cbl det validity bit bugs 
>>> 	*/
>>> 	ATA_HORKAGE_STUCK_ERR	= (1 << 9),	/* stuck ERR on next PACKET 
>>> 	*/
>>> +	ATA_HORKAGE_BRIDGE_OK	= (1 << 10),	/* no bridge limits */
>>>
>>> 	 /* DMA mask for user DMA control: User visible values; DO NOT
>>> 	    renumber */
>>
>> This one seems fine to me...
> 
> OK, so here's a version that I also tested. I applied the bridge ok flag
> to MTRON MSP-SATA, that is the closest match to the 70xx and 75xx
> series.
> 
> ---
> 
> From: Jens Axboe <jens.axboe@oracle.com>
> Subject: [PATCH] libata: add whitelist for devices with known good pata-sata bridges
> 
> libata currently imposes a UDMA5 max transfer rate and 200 sector max
> transfer size for SATA devices that sit behind a pata-sata bridge. Lots
> of devices have known good bridges that don't need this limit applied.
> The MTRON SSD disks are such devices. Transfer rates are increased by
> 20-30% with the restriction removed.
> 
> So add a "blacklist" entry for the MTRON devices, with a flag indicating
> that the bridge is known good.

applied