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

["René Rebe" <rene@exactcode.com>]

Hi,

> On 25. Apr 2021, at 20:25, Hans de Goede <hdegoede@redhat.com> wrote:
> 
> Hi Rene,
> 
> On 4/25/21 5:02 PM, Rene Rebe wrote:
>> Alan Stern <stern@rowland.harvard.edu> wrote:
>> 
>>> On Sun, Apr 25, 2021 at 02:15:36PM +0200, Rene Rebe wrote:
>>>> From: Greg KH <gregkh@linuxfoundation.org>
>>>> Subject: Re: [PATCH] unbreak all modern Seagate ATA pass-through for SMART
>>>> Date: Sun, 25 Apr 2021 14:00:26 +0200
>>>> 
>>>>>> I would expect that more modern devices to work. Vendors usually
>>>>>> linearly allocate their product ids for new devices, and we could
>>>>>> allow list product ids higher than this Seven to unbreak more modern
>>>>>> devices by default and limit the amount of device quirks needed?
>>>>> 
>>>>> Vendors do not allocate device ids that way at all, as there is no
>>>>> requirement to do so.  I know of many vendors that seemingly use random
>>>>> values from their product id space, so there is no guarantee that this
>>>>> will work, sorry.
>>>> 
>>>> I did not say it is a requirement, just that they usually do speaking
>>>> of just this Seagate case. What is wrong with using that to
>>>> potentially significantly cut down the quirk list?
>>> 
>>> You didn't read commit 92335ad9e895, did you?  It lists a large number 
>>> of Seagate devices that don't work with ATA pass-through, and three of 
>>> them have product IDs that are larger than 0xab03.
>>> 
>>> Please check the facts before guessing about things that will cause 
>>> problems for other people.
>> 
>> I really don't appreciate the unfriendly tone on the linux kernel
>> mailing lists (which is why for 20 years I never felt like contributing
>> here mo),
> 
> I'm sorry to hear that the admittently sometimes unfriendly tone
> on the kernel mailinglists have stopped you from contributing.
> 
> Note that I do believe that things have gotten better recently.
> 
> As for this email-thread, I don't really see anything wrong with
> Alan's reply here. You have been quite argumentative in this entire
> thread about how things would be much better if they are done your
> way.
> 
> I cannot speak for the others but I certainly have gotten annoyed by
> the tone of your emails so far, I apologize if any of that annoyance
> has bleed through in the tone of my emails. I do strive to always
> stay professional (but as all of us I'm only human).
> 
> <snip>
> 
>>>>> What is wrong with just allowing specific devices that you have tested
>>>>> will work, to the list instead?  That's the safest way to handle this.
>>>> 
>>>> The problem is that out of the box it does not work for users, and
>>>> normal users do not dive into the kernel code to find out and simply
>>>> think their devices sucks. Even I for years thought the drive sucks,
>>>> before I took a closer look. If you long term want more new devices in
>>>> an allow list than the previous quirk list included (9 or 10 does not
>>>> sound that many to me), ... whatever you prefer ,-) I would rather
>>>> have 10 quirk disable list than potential many more white listed the
>>>> next years to come. Maybe we shoudl simply restore the prevoius
>>>> quirks.
>>> 
>>> That's a possibility, and in the future we may do it.  But probably not 
>>> until the enable list grows to a comparable length.
>> 
>> Yeah, why future proof it now, ...
> 
> Perhaps look in the mirror and start with improving the tone of your
> own emails ?

Thanks, I re-read them and find them pretty ok.

>> which is exactly what brought us
>> here from the original regression. The enable list will likely not
>> grow quickly as most users will just expect a broken device hw/
>> firmware and not bother looking into it and instead live w/o SMART.
> 
> Right because people can happily live without SMART most users won't
> even know they're missing some optional functionality. Non working disks
> OTOH will lead to bug reports, bug reports of which Alan, Greg and I
> will be on the receiving end.
> 
> Moreover the kernel has a very strong no regressions policy, and what
> you suggest would almost certainly break stuff for some of our users,
> so we can simply not do as you suggest.
> 
> I notice that you call the lack of SMART support on your own disk
> a regression, but that has never worked with any recent kernel
> (due to the discussed code which has been present since 2017) so
> from the no-regressions kernel policy pov that is not a regression.

Well, it was working before, and does not since the quoted commit.
I call that a regression. Weather that was recently or some years
ago does not change the definition of regression IMHO.

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

Plus a dedicated page about this change and devices stopped working:

	https://www.smartmontools.org/wiki/SAT-with-UAS-Linux

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

Have a great day,
	René Rebe

-- 
 ExactCODE GmbH, Lietzenburger Str. 42, DE-10789 Berlin, https://exactcode.com
 https://exactscan.com | https://ocrkit.com | https://t2sde.org | https://rene.rebe.de