Re: [PATCH] unbreak all modern Seagate ATA pass-through for SMART

Hans de Goede
2021-04-26T09:54:22Z

Hi,

On 4/26/21 11:40 AM, Rene Rebe wrote:
> From: Hans de Goede <hdegoede@redhat.com>
> Date: Mon, 26 Apr 2021 10:16:12 +0200
>
>> Hi,
>>
>> On 4/25/21 10:52 PM, René Rebe wrote:

<snip>

>> Ideally we could just wave a magic wand and make this all work,
>> but unfortunately reality is not cooperating, so we need to come up
>> with some pragmatic solution here.
>
> I did not mean to be compative, however, as usual in real life we just
> do not agree with all the reasoning ;-)

Yes, thank you for understanding.

>>> On further internet searching that there are at least 4 more drivers
>>> listed on the smartmontools page that should work:
>>>
>>> https://www.smartmontools.org/wiki/Supported_USB-Devices
>>
>> That is a very interesting link thank you. That certainly advocates
>> for a generic approach introducing a new US_FL_ATA_1X_OK and then
>> adding quirks setting that for both your model and the 4 models
>> listed there.
>>
>> I would really appreciate it if you could submit a patch series
>> for this. But if you don't want to do that then I'll put this on
>> my own TODO list.
>
> Maybe another week, however, as this is not the semantic I prefer that
> would only cause more work for me with a bigger reverting patch in our
> tree at the end, ...

Thank you for considering working on this. If you decide not to do
this in the end, please let me know then I'll add this to my
(way too long) TODO list.

> <snip>
>
>>> Given this, I will not continue spending my time on implementing
>>> what you suggested and instead simply reverted this for our Linux
>>> SDE as I believe results in the best out of the box experience
>>> for our users:
>>>
>>> https://svn.exactcode.de/t2/trunk/package/base/linux/uas-seagate.patch
>>
>> I've taken a quick peek at this and I see that you've also restored
>> the old per model quirks to disable ATA pass-through on known to be
>> broken models, good.
>
> Yes, I reverted that, and added two more I found from the old email
> thread that probably triggered the code change back in the day.

Ah I was already why there were more entries then I expected,
good job.

>> Note that the list of broken models which you've added it missing the
>> 0xab25 and 0xab38 product-ids which according to:
>> https://www.smartmontools.org/wiki/Supported_USB-Devices
>> have broken ATA passthrough support with UAS.
>
> Thanks, I added those two now as well.

Great, that means your patch will be a good starting point for
the broken devices list if we do ever decide to flip the
default for Seagate devices back to allowing ATA pass-through by
default.

>> If I assume that these behave as some of the other Seagate drivers and
>> the bridge-chip crashes when trying to use ATA pass-through, then once
>> these changes hit your users and customers you have just broken usage
>> of those disks together with your product. This nicely illustrates
>> why we don't want to make this change in the mainline kernel.
>>
>> Note depending on how important disk performance is for you
>> an alternative approach might be to modify the Seagate product-id check
>> to simply disable UAS on Seagate devices, that would be a lot safer.
>
> We do not run a smartd by default>

Ah yes that helps, unfortunately in the mainline kernel we cannot
assume that that is the case.

> and I actually prefer a driver that
> deaults to behave by the standard book, and get notified when
> something goes wrong, instead of globally disallow listing a whole
> vendor.
>
> Maybe it is still an option to restore the updated unusual quirk list
> entries, that way users with newer devices get their SMART back sooner
> than later and this also encourages Seagate to continue producing
> fully working devices, without hiding any ATA pass-through bugs by
> default ;-)

It is always an option :) I just don't think that this is the right
moment in time to do it. Notice that your email archive digging +
the https://www.smartmontools.org/wiki/Supported_USB-Devices have
turned op 4 new broken devices for which we did not have quirks
before. I'm simply afraid that that is just the tip of the iceberg.
Causing peoples disks to stop working is not just a bug, it is
a very very bad bug, so yeah I'm quite conservative here, sorry.

So for now I believe that the best thing we can do is to agree
that we disagree on the best way to handle this.

(Now if only I had this magic-wand which could give me the complete
list of all broken models, then things would be different.)

Regards,

Hans