linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: James Bottomley <James.Bottomley@suse.de>
To: Constantin Baranov <const@mimas.ru>
Cc: linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: [PATCH] hwmon: Driver for SCSI/ATA temperature sensors
Date: Sun, 13 Sep 2009 09:00:53 -0500	[thread overview]
Message-ID: <1252850453.23599.13.camel@mulgrave.site> (raw)
In-Reply-To: <20090913040104.ab1d0b69.const@mimas.ru>

On Sun, 2009-09-13 at 04:01 +0500, Constantin Baranov wrote:
> The scsitemp module attaches a device to each SCSI device
> and registers it in hwmon. Currently the only method of
> reading temperature is ATA SMART. Adding support of the
> pure SCSI methods is provided.

The code, as you wrote it looks fine.

The basic problem are the effects.  Right at the moment it tries to send
an ATA_16 encapsulated SMART command to every SCSI device in the system.
We simply can't allow this.  A huge number of SCSI presenting USB
devices are known to lock up when they see either ATA_X encapsulation or
SMART commands.  It's not really even safe to send ATA_X to SCSI
devices.  The modern ones should all error out fine, but the older ones
are likely to be less tolerant.

Finally, this:

> +               if (attr[2] == 194) {
> +                       *temp = attr[7] * 1000;
> +                       err = 0;
> +                       break;

Smart attribute 194 is highly vendor specific ... for instance my new
SATA drive doesn't implement it (it does implement 190 instead).

So really, given the complexity of just obtaining the data and the
problem of matching which data to obtain to which device you have, why
not just use smartctl from userspace for monitoring?  you could even
just plug into smartd, it seems to have most of the necessary heuristics
built in already.

James



  parent reply	other threads:[~2009-09-13 14:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-09-12 23:01 [PATCH] hwmon: Driver for SCSI/ATA temperature sensors Constantin Baranov
2009-09-13  8:04 ` Rolf Eike Beer
2009-09-13 11:56   ` Constantin Baranov
2009-09-13 14:00 ` James Bottomley [this message]
2009-09-14 15:00   ` Pavel Machek
2009-09-15  5:57   ` Julian Calaby
2009-09-16 17:43     ` Constantin Baranov

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=1252850453.23599.13.camel@mulgrave.site \
    --to=james.bottomley@suse.de \
    --cc=const@mimas.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    /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).