linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-hwmon@vger.kernel.org, Jean Delvare <jdelvare@suse.com>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org
Subject: Re: [PATCH 0/1] Summary: hwmon driver for temperature sensors on SATA drives
Date: Mon, 16 Dec 2019 21:35:33 -0500	[thread overview]
Message-ID: <yq1y2vbhe6i.fsf@oracle.com> (raw)
In-Reply-To: <67b75394-801d-ce91-55f2-f0c0db9cfffc@roeck-us.net> (Guenter Roeck's message of "Tue, 10 Dec 2019 21:57:24 -0800")


Guenter,

> If and when drives are detected which report bad information, such
> drives can be added to a blacklist without impact on the core SCSI or
> ATA code. Until that happens, not loading the driver solves the
> problem on any affected system.

My only concern with that is that we'll have blacklisting several
places. We already have ATA and SCSI blacklists. If we now add a third
place, that's going to be a maintenance nightmare.

More on that below.

>> My concerns are wrt. identifying whether SMART data is available for
>> USB/UAS. I am not too worried about ATA and "real" SCSI (ignoring RAID
>> controllers that hide the real drives in various ways).

OK, so I spent my weekend tinkering with 15+ years of accumulated USB
devices. And my conclusion is that no, we can't in any sensible manner,
support USB storage monitoring in the kernel. There is no heuristic that
I can find that identifies that "this is a hard drive or an SSD and
attempting one of the various SMART methods may be safe". As opposed to
"this is a USB key that's likely to lock up if you try". And that's
ignoring the drives with USB-ATA bridges that I managed to wedge in my
attempt at sending down commands.

Even smartmontools is failing to work on a huge part of my vintage
collection.  Thanks to a wide variety of bridges with random, custom
interfaces.

So my stance on all this is that I'm fine with your general approach for
ATA. I will post a patch adding the required bits for SCSI. And if a
device does not implement either of the two standard methods, people
should use smartmontools.

Wrt. name, since I've added SCSI support, satatemp is a bit of a
misnomer. drivetemp, maybe? No particular preference.

> The one USB/UAS connected SATA drive I have (a WD passport) reports
> itself as "WD      ", not as "ATA     ". I would expect other drives
> to do the same.

Yes. Most vendors are too fond of their brand names to put "ATA" in
there. So my suggestion is to relax the heuristic to trigger on the ATA
Information VPD page only and ignore the name.

Also, there are some devices that will lock up the way you access that
VPD page. So a tweak is also required there.

To avoid the multiple blacklists and heuristic collections my suggestion
is that I introduce a helper function in SCSI (based on what I did in
the disk driver) that can be called to identify whether something is an
ATA device. And then the hwmon driver can call that and we can keep the
heuristics in one place.

If a device turns out to be problematic wrt. getting the ATA VPD for the
purpose of SMART, for instance, it will also need to be blacklisted for
other reasons in SCSI. So I would really like to keep the heuristics in
one place.

-- 
Martin K. Petersen	Oracle Linux Engineering

  reply	other threads:[~2019-12-17  2:38 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-09  5:21 [PATCH 0/1] Summary: hwmon driver for temperature sensors on SATA drives Guenter Roeck
2019-12-09  5:21 ` [PATCH 1/1] hwmon: Driver " Guenter Roeck
2019-12-09  5:28   ` Randy Dunlap
2019-12-09  6:00     ` Guenter Roeck
2019-12-09 17:08   ` Bart Van Assche
2019-12-09 19:20     ` Guenter Roeck
2019-12-10 16:10       ` Bart Van Assche
2019-12-12 22:33   ` Linus Walleij
2019-12-12 23:21     ` Martin K. Petersen
2019-12-13  4:18       ` Guenter Roeck
2019-12-17  2:47         ` Martin K. Petersen
2019-12-17  4:20           ` Guenter Roeck
2019-12-18  3:39             ` Martin K. Petersen
2019-12-11  4:08 ` [PATCH 0/1] Summary: hwmon driver " Martin K. Petersen
2019-12-11  5:57   ` Guenter Roeck
2019-12-17  2:35     ` Martin K. Petersen [this message]
2019-12-17  3:57       ` Guenter Roeck
2019-12-17  5:50         ` Damien Le Moal
2019-12-17 15:47           ` Guenter Roeck
2019-12-18  3:42         ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=yq1y2vbhe6i.fsf@oracle.com \
    --to=martin.petersen@oracle.com \
    --cc=jdelvare@suse.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux@roeck-us.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).