Re: [PATCH] libata-core More robust parsing for multi_count(v5)

[Mark Lord <liml@rtr.ca>]

Tejun Heo wrote:
> Hello, Mark.
> 
> Mark Lord wrote:
>> Make libata more robust when parsing the multi_count
>> field from a drive's identify data.  This prevents us from
>> attempting to use dubious multi_count values ad infinitum.
>>
>> Reset dev->multi_count to zero and reprobe it each time
>> through this routine, as it can change on device reset.
>>
>> Also ensure that the reported "maximum" value is valid
>> and is a power of two, and that the reported "count" value
>> is valid and also a power of two.  And that the "count"
>> value is not greater than the "maximum" value.
>>
>> Signed-off-by: Mark Lord <mlord@pobox.com>
> 
> Acked-by: Tejun Heo <tj@kernel.org>
> 
> The patch looks safe to me although requiring power_of_2 on max seems
> a bit too strict.
..

Just a safeguard against bad IDENTIFY data.
I've never seen a drive that specified anything there
other than 1, 8, or 16,  and the ATA spec specifically
recommends against using settings that are not powers of 2.

> For configuration, ata_dev_configure() is the right
> place.  I think the following should work.
> 
> * add dev->multi_limit which is initialized to -1 on ata_dev_init().
> 
> * during ata_dev_configure() if multi_limit < 0, set it to device max.
> 
> * set dev->multi to rounddown_pow_of_two(multi_limit) and issue
>   SET_MULTI.
> 
> On EH side, ata_eh_speed_down() is the place to handle it.  It can
> probably piggy back on ATA_EH_SPDN_SPEED_DOWN block.  All it has to do
> is reducing dev->multi_limit.  dev_config will run later and apply it.
..

I won't have time to pursue this in the near/foreseeable future,
so if you feel the urge.. please do it.  Otherwise I might get to
it in a few months from now.

> I'm not sure how the speed down strategy should exactly be tho.  It is
> distinguisible from regular transfer errors?
..

I don't really have a recommendation there, other than to simply stop
using multi_count if there are repeated PIO errors.  Seems like a reasonable
strategy, when all else is failing.

We really need to add SET_MULTIPLE smarts to libata at some point.

Cheers