Skip to contentSkip to conversation

libata: implement and use ata_dev_revalidate()

PATCH In review Tejun Heo 2 participants 9 messages Feb 15, 2006 – Feb 20, 2006

Subsystems (2)
lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 libata: implement and use ata_dev_revalidate()

Hello, Jeff. Hello, Albert.

This patchset is the fourth and last part of divided 'reorganize configuration and implement revalidation' patchset[1]. The third part was posted about half an hour ago[2].

This patchset is against
the current upstream (254a5c45f8e53775a1e1dd7498a8dcd665341f1e)
+ rename ata_dev_id_[c_]string() patch [3]
+ reorganize ata_dev_identify() patchset [2]

Note: Jeff, I generated patches over the rename patch as the patch
make naming not only shorter but also more consistent. If you don't like the rename, I'll regenerate this patchset without it.

This patchset implements ata_dev_revalidate() and use it after configuring transfer mode.

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.ide/7632
[2] http://article.gmane.org/gmane.linux.ide/8048
[3] http://article.gmane.org/gmane.linux.ide/7997

author
#1 lore raw

PATCH 1/4 libata: re-initialize parameters before configuring

In ata_dev_configure(), reinitialize parameters before configuring. This change is for revalidation and hotplug. As ata_dev_configure() can be entered multiple times, parameters need to be reinitialized.

---

 drivers/scsi/libata-core.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

1f2bfa33d83b17398e8207a2fa0926f7454592de

drivers/scsi/libata-core.c +9 -0
index 8c0d48f..abb08c5 100644
@@ -1082,6 +1082,15 @@ static int ata_dev_configure(struct ata_
 
 	DPRINTK("ENTER, host %u, dev %u\n", ap->id, dev->devno);
 
+	/* initialize to-be-configured parameters */
+	dev->flags = 0;
+	dev->max_sectors = 0;
+	dev->cdb_len = 0;
+	dev->n_sectors = 0;
+	dev->cylinders = 0;
+	dev->heads = 0;
+	dev->sectors = 0;
+
 	/*
 	 * common ATA, ATAPI feature tests
 	 */
Signature · 1 line
1.1.5
#2 lore raw

Re: [PATCH 1/4] libata: re-initialize parameters before configuring

Quoted 5 lines · Tejun Heo

In ata_dev_configure(), reinitialize parameters before configuring. This change is for revalidation and hotplug. As ata_dev_configure() can be entered multiple times, parameters need to be reinitialized.

ACK

author
#3 lore raw

PATCH 3/4 libata: implement ata_dev_revalidate()

In reply to Tejun Heo

ata_dev_revalidate() re-reads IDENTIFY PAGE of the given device and makes sure it's the same device as the configured one. Once it's verified that it's the same device, @dev is configured according to newly read IDENTIFY PAGE. Note that revalidation currently doesn't invoke transfer mode reconfiguration.

Criteria for 'same device'

* same class (of course)
* same model string
* same serial string
* if ATA, same n_sectors (to catch geometry parameter changes)

---

 drivers/scsi/libata-core.c |  115 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h     |    2 +
 2 files changed, 117 insertions(+), 0 deletions(-)

95213b21c681afd2eef1e5cbae21337208ac4c46

drivers/scsi/libata-core.c +115 -0
index 05dadf5..54ed8fd 100644
@@ -2341,6 +2341,120 @@ int ata_drive_probe_reset(struct ata_por
 	return rc;
 }
 
+/**
+ *	ata_dev_same_device - Determine whether new ID matches configured device
+ *	@ap: port on which the device to compare against resides
+ *	@dev: device to compare against
+ *	@new_class: class of the new device
+ *	@new_id: IDENTIFY page of the new device
+ *
+ *	Compare @new_class and @new_id against @dev and determine
+ *	whether @dev is the device indicated by @new_class and
+ *	@new_id.
+ *
+ *	LOCKING:
+ *	None.
+ *
+ *	RETURNS:
+ *	1 if @dev matches @new_class and @new_id, 0 otherwise.
+ */
+static int ata_dev_same_device(struct ata_port *ap, struct ata_device *dev,
+			       unsigned int new_class, const u16 *new_id)
+{
+	const u16 *old_id = dev->id;
+	unsigned char model[2][41], serial[2][21];
+	u64 new_n_sectors;
+
+	if (dev->class != new_class) {
+		printk(KERN_WARNING
+		       "ata%u: dev %u class mismatch %d != %d\n",
+		       ap->id, dev->devno, dev->class, new_class);
+		return 0;
+	}
+
+	ata_id_c_string(old_id, model[0], ATA_ID_PROD_OFS, sizeof(model[0]));
+	ata_id_c_string(new_id, model[1], ATA_ID_PROD_OFS, sizeof(model[1]));
+	ata_id_c_string(old_id, serial[0], ATA_ID_SERNO_OFS, sizeof(serial[0]));
+	ata_id_c_string(new_id, serial[1], ATA_ID_SERNO_OFS, sizeof(serial[1]));
+	new_n_sectors = ata_id_n_sectors(new_id);
+
+	if (strcmp(model[0], model[1])) {
+		printk(KERN_WARNING
+		       "ata%u: dev %u model number mismatch '%s' != '%s'\n",
+		       ap->id, dev->devno, model[0], model[1]);
+		return 0;
+	}
+
+	if (strcmp(serial[0], serial[1])) {
+		printk(KERN_WARNING
+		       "ata%u: dev %u serial number mismatch '%s' != '%s'\n",
+		       ap->id, dev->devno, serial[0], serial[1]);
+		return 0;
+	}
+
+	if (dev->class == ATA_DEV_ATA && dev->n_sectors != new_n_sectors) {
+		printk(KERN_WARNING
+		       "ata%u: dev %u n_sectors mismatch %llu != %llu\n",
+		       ap->id, dev->devno, (unsigned long long)dev->n_sectors,
+		       (unsigned long long)new_n_sectors);
+		return 0;
+	}
+
+	return 1;
+}
+
+/**
+ *	ata_dev_revalidate - Revalidate ATA device
+ *	@ap: port on which the device to revalidate resides
+ *	@dev: device to revalidate
+ *	@post_reset: is this revalidation after reset?
+ *
+ *	Re-read IDENTIFY page and make sure @dev is still attached to
+ *	the port.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep)
+ *
+ *	RETURNS:
+ *	0 on success, negative errno otherwise
+ */
+int ata_dev_revalidate(struct ata_port *ap, struct ata_device *dev,
+		       int post_reset)
+{
+	unsigned int class;
+	u16 *id;
+	int rc;
+
+	if (!ata_dev_present(dev))
+		return 0;
+
+	class = dev->class;
+	id = NULL;
+
+	/* allocate & read ID data */
+	rc = ata_dev_read_id(ap, dev, &class, post_reset, &id);
+	if (rc)
+		goto fail;
+
+	/* is the device still there? */
+	if (!ata_dev_same_device(ap, dev, class, id)) {
+		rc = -ENODEV;
+		goto fail;
+	}
+
+	kfree(dev->id);
+	dev->id = id;
+
+	/* configure device according to the new ID */
+	return ata_dev_configure(ap, dev, 0);
+
+ fail:
+	printk(KERN_ERR "ata%u: dev %u revalidation failed (errno=%d)\n",
+	       ap->id, dev->devno, rc);
+	kfree(id);
+	return rc;
+}
+
 static void ata_pr_blacklisted(const struct ata_port *ap,
 			       const struct ata_device *dev)
 {
@@ -4956,6 +5070,7 @@ EXPORT_SYMBOL_GPL(sata_std_hardreset);
 EXPORT_SYMBOL_GPL(ata_std_postreset);
 EXPORT_SYMBOL_GPL(ata_std_probe_reset);
 EXPORT_SYMBOL_GPL(ata_drive_probe_reset);
+EXPORT_SYMBOL_GPL(ata_dev_revalidate);
 EXPORT_SYMBOL_GPL(ata_port_disable);
 EXPORT_SYMBOL_GPL(ata_ratelimit);
 EXPORT_SYMBOL_GPL(ata_busy_sleep);
include/linux/libata.h +2 -0
index e8e02a0..81014be 100644
@@ -485,6 +485,8 @@ extern int ata_std_softreset(struct ata_
 extern int sata_std_hardreset(struct ata_port *ap, int verbose,
 			      unsigned int *class);
 extern void ata_std_postreset(struct ata_port *ap, unsigned int *classes);
+extern int ata_dev_revalidate(struct ata_port *ap, struct ata_device *dev,
+			      int post_reset);
 extern void ata_port_disable(struct ata_port *);
 extern void ata_std_ports(struct ata_ioports *ioaddr);
 #ifdef CONFIG_PCI
Signature · 1 line
1.1.5
author
#4 lore raw

PATCH 4/4 libata: revalidate after transfer mode configuration

In reply to Tejun Heo

Revalidate device after transfer mode configuration. This also makes dev->id up-to-date.

---

 drivers/scsi/libata-core.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

4580280ce48d62d2d801cd427e6194f9ab56e84b

drivers/scsi/libata-core.c +6 -0
index 54ed8fd..c971c15 100644
@@ -1620,6 +1620,12 @@ static void ata_dev_set_mode(struct ata_
 	idx = ofs + dev->xfer_shift;
 	WARN_ON(idx >= ARRAY_SIZE(xfer_mode_str));
 
+	if (ata_dev_revalidate(ap, dev, 0)) {
+		printk(KERN_ERR "ata%u: failed to revalidate after set "
+		       "xfermode, disabled\n", ap->id);
+		ata_port_disable(ap);
+	}
+
 	DPRINTK("idx=%d xfer_shift=%u, xfer_mode=0x%x, base=0x%x, offset=%d\n",
 		idx, dev->xfer_shift, (int)dev->xfer_mode, (int)base, ofs);
 
Signature · 1 line
1.1.5
#5 lore raw

Re: [PATCH 4/4] libata: revalidate after transfer mode configuration

Quoted 16 lines · Tejun Heo

Revalidate device after transfer mode configuration. This also makes dev->id up-to-date.

---

 drivers/scsi/libata-core.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

4580280ce48d62d2d801cd427e6194f9ab56e84b

drivers/scsi/libata-core.c +0 -0
index 54ed8fd..c971c15 100644

drivers/scsi/libata-core.c static void ata_dev_set_mode(struct ata_ · line 1627

Quoted 8 lines
drivers/scsi/libata-core.c +5 -0
@@ -1620,6 +1620,12 @@ static void ata_dev_set_mode(struct ata_
 	idx = ofs + dev->xfer_shift;
 	WARN_ON(idx >= ARRAY_SIZE(xfer_mode_str));
 
+	if (ata_dev_revalidate(ap, dev, 0)) {
+		printk(KERN_ERR "ata%u: failed to revalidate after set "
+		       "xfermode, disabled\n", ap->id);
+		ata_port_disable(ap);
+	}

why disable the entire port on error?

Jeff

author
#6 lore raw

Re: [PATCH 4/4] libata: revalidate after transfer mode configuration

Quoted 21 lines · Jeff Garzik
Quoted 19 lines · Tejun Heo

Revalidate device after transfer mode configuration. This also makes dev->id up-to-date.

---

 drivers/scsi/libata-core.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

4580280ce48d62d2d801cd427e6194f9ab56e84b

drivers/scsi/libata-core.c +0 -0
index 54ed8fd..c971c15 100644
@@ -1620,6 +1620,12 @@ static void ata_dev_set_mode(struct ata_
     idx = ofs + dev->xfer_shift;
     WARN_ON(idx >= ARRAY_SIZE(xfer_mode_str));
 
Quoted 9 lines

drivers/scsi/libata-core.c static void ata_dev_set_mode(struct ata_ · line 1627

Quoted 5 lines
drivers/scsi/libata-core.c +5 -0
+    if (ata_dev_revalidate(ap, dev, 0)) {
+        printk(KERN_ERR "ata%u: failed to revalidate after set "
+               "xfermode, disabled\n", ap->id);
+        ata_port_disable(ap);
+    }

why disable the entire port on error?

Because that's what ata_set_mode() and its descendants does currently. They disable port on failure. I didn't want to introduce new behavior with this patch. The port disable -> device disable conversion will be done in later patch series together with all other parts of ata_set_mode(). No?

Signature · 1 line
tejun
#7 lore raw

Re: [PATCH 4/4] libata: revalidate after transfer mode configuration

Quoted 34 lines · Tejun Heo
Quoted 21 lines · Jeff Garzik
Quoted 19 lines · Tejun Heo

Revalidate device after transfer mode configuration. This also makes dev->id up-to-date.

---

 drivers/scsi/libata-core.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

4580280ce48d62d2d801cd427e6194f9ab56e84b

drivers/scsi/libata-core.c +0 -0
index 54ed8fd..c971c15 100644
@@ -1620,6 +1620,12 @@ static void ata_dev_set_mode(struct ata_
     idx = ofs + dev->xfer_shift;
     WARN_ON(idx >= ARRAY_SIZE(xfer_mode_str));
 
Quoted 10 lines

drivers/scsi/libata-core.c static void ata_dev_set_mode(struct ata_ · line 1627

Quoted 5 lines
drivers/scsi/libata-core.c +5 -0
+    if (ata_dev_revalidate(ap, dev, 0)) {
+        printk(KERN_ERR "ata%u: failed to revalidate after set "
+               "xfermode, disabled\n", ap->id);
+        ata_port_disable(ap);
+    }

why disable the entire port on error?

Quoted 1 line

Because that's what ata_set_mode() and its descendants does currently. They disable port on failure. I didn't want to introduce new behavior with this patch. The port disable -> device disable conversion will be done in later patch series together with all other parts of ata_set_mode(). No?

OK

#8 lore raw
In reply to Tejun Heo
Quoted 11 lines · Tejun Heo

Hello, Jeff. Hello, Albert.

This patchset is the fourth and last part of divided 'reorganize configuration and implement revalidation' patchset[1]. The third part was posted about half an hour ago[2].

This patchset is against
the current upstream (254a5c45f8e53775a1e1dd7498a8dcd665341f1e)
+ rename ata_dev_id_[c_]string() patch [3]
+ reorganize ata_dev_identify() patchset [2]

Quoted 6 lines

Note: Jeff, I generated patches over the rename patch as the patch
make naming not only shorter but also more consistent. If you don't like the rename, I'll regenerate this patchset without it.

This patchset implements ata_dev_revalidate() and use it after configuring transfer mode.

In general the patchset is OK, but there are comments... (see forthcoming emails)

Jeff