linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Lord <liml@rtr.ca>
To: Jeff Garzik <jgarzik@pobox.com>, Tejun Heo <htejun@gmail.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: IDE/ATA development list <linux-ide@vger.kernel.org>
Subject: [PATCH] libata-core Use more robust parsing for multi_count
Date: Wed, 18 Mar 2009 10:26:05 -0400	[thread overview]
Message-ID: <49C1047D.4000008@rtr.ca> (raw)

Gents,

I am debugging a drive hang issue for a client,
which involves "multiple sector" read/write commands (PIO).

Libata does not appear (as of current #upstream at least)
to correctly implement this functionality.

Specifically, libata doesn't reset dev->multi_count to zero
on drive resets (whereas the drive firmware often *does*),
and libata does not deal well with dubious IDENTIFY data.

So, on resets, we should either clear dev->multi_count,
or re-probe it from the drive, or issue a SET_MULTIPLE command
to restore the old value.

When probing:

If (dev->id[59] == 0xffff), ignore it.
If (dev->id[59] & 0x00ff) is not an even power of two, ignore it.
it should also be ignored (this catches the 0xffff case too).

The powers of two is from the ATA8 standard, which strongly suggests
that only powers of two are valid for use with SET_MULTIPLE.

The probe stuff is trivial to fix (below), but I'm not really
up to speed on figuring out how to get this code re-invoked
after drive resets, and on how to ensure that a SET_MULTIPLE
gets issued to restore the previous working value (with eventual
fallback to not using it after repeated errors).   Tejun?

<--- snip --->

Make libata a little more robust in parsing the multi_count
field from a drive's identify data.  This prevents libata from
attempting to use dubious multi_count values ad infinitum.

Signed-off-by: Mark Lord <mlord@pobox.com>

--- upstream/drivers/ata/libata-core.c	2009-03-18 10:21:07.000000000 -0400
+++ new/drivers/ata/libata-core.c	2009-03-18 10:22:05.000000000 -0400
@@ -2426,9 +2426,20 @@
 
 		dev->n_sectors = ata_id_n_sectors(id);
 
-		if (dev->id[59] & 0x100)
-			dev->multi_count = dev->id[59] & 0xff;
-
+		dev->multi_count = 0;
+		if (dev->id[59] & 0x100) {
+			switch (dev->id[59] & 0xff) {
+			case 1:
+			case 2:
+			case 4:
+			case 8:
+			case 16:
+			case 32:
+			case 64:
+			case 128:
+				dev->multi_count = dev->id[59] & 0xff;
+			}
+		}
 		if (ata_id_has_lba(id)) {
 			const char *lba_desc;
 			char ncq_desc[20];

             reply	other threads:[~2009-03-18 14:26 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-18 14:26 Mark Lord [this message]
2009-03-18 14:32 ` [PATCH] libata-core Use more robust parsing for multi_count Alan Cox
2009-03-18 15:06   ` Mark Lord
2009-03-18 15:13     ` Mark Lord
2009-03-18 17:09     ` Alan Cox
2009-03-18 15:58 ` Mark Lord
2009-03-18 16:18   ` [PATCH] libata-core More robust parsing for multi_count(v3) Mark Lord
2009-03-18 16:24     ` Mark Lord
2009-03-19  0:23     ` Tejun Heo
2009-03-19  0:25       ` Tejun Heo
2009-03-19 17:30         ` [PATCH] libata-core More robust parsing for multi_count(v4) Mark Lord
2009-03-19 17:32           ` [PATCH] libata-core More robust parsing for multi_count(v5) Mark Lord
2009-03-19 23:33             ` Tejun Heo
2009-03-20  3:37               ` Mark Lord
2009-03-20 13:13               ` Mark Lord
2009-03-20 13:14                 ` Mark Lord
2009-03-20 14:07                   ` Alan Cox
2009-03-20 15:36                     ` Mark Lord
2009-03-20 23:14                       ` Tejun Heo
2009-03-21  0:54                         ` Jeff Garzik
2009-03-21  2:17                           ` Tejun Heo
2009-03-21 13:54                             ` Mark Lord
2009-03-21 14:02                           ` Alan Cox
2009-03-21 14:59                             ` Mark Lord
2009-03-20 13:38                 ` Alan Cox
2009-04-12 15:10                 ` Mark Lord
2009-04-12 15:18                   ` Alan Cox
2009-04-12 15:31                     ` Jeff Garzik
2009-03-25  2:40             ` Jeff Garzik

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=49C1047D.4000008@rtr.ca \
    --to=liml@rtr.ca \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=htejun@gmail.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@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).