libata: implement and use ata_dev_revalidate(), take #2
PATCH 1/1 libata: implement and use ata_dev_revalidate(), take #2
Hello, Jeff.
This is the second take of implement-and-use-ata_dev_revalidate patchset. In the last take[1], out of four patches, %1 got ACK'ed, %2 had some minor issues, %3 got no comment and %4 had some issue but got resolved.
Changes from the last take are...
* in #2, @verbose is renamed to @print_info as suggested
* in #2, lba_desc change is described in the patch description
This patchset is against
the current #upstream[2]
+ kill-illegal-kfree patch[3]
+ reorganize-ata_dev_identify patchset[4]
Thanks.
--
tejun
[1] http://article.gmane.org/gmane.linux.ide/8050
[2] cccc65a3b60edaf721cdee5a14f68ba009341822
[3] http://article.gmane.org/gmane.linux.ide/8324
[4] http://article.gmane.org/gmane.linux.ide/8412
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(-)
ce0cef100691a38180b32a74ce6ab7ca90b390f0
drivers/scsi/libata-core.c +9 -0
@@ -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.2.1
PATCH 3/4 libata: implement ata_dev_revalidate()
In reply to Tejun Heoata_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(-)
5f157f520bce43bee189d18e0736065ce7997334
drivers/scsi/libata-core.c +115 -0
@@ -2341,6 +2341,120 @@ int ata_drive_probe_reset(struct ata_porreturn 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) {
@@ -4960,6 +5074,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
@@ -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.2.1
Re: [PATCH 3/4] libata: implement ata_dev_revalidate()
Quoted 80 lines · 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, samen_sectors(to catch geometry parameter changes)Signed-off-by Tejun Heo <[email protected]>---
drivers/scsi/libata-core.c | 115 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/libata.h | 2 + 2 files changed, 117 insertions(+), 0 deletions(-)
5f157f520bce43bee189d18e0736065ce7997334drivers/scsi/libata-core.c +50
-0@@ -2341,6 +2341,120 @@ int ata_drive_probe_reset(struct ata_porreturn 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 ; + }
drivers/scsi/libata-core.c int ata_drive_probe_reset(struct ata_por · line 2403
Quoted 10 lines
drivers/scsi/libata-core.c +10
-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 ;
For the hotplug case, its inaccurate to call these warnings.
Quoted 13 lines
drivers/scsi/libata-core.c +13
-0+* 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
drivers/scsi/libata-core.c int ata_drive_probe_reset(struct ata_por · line 2429
Quoted 10 lines
drivers/scsi/libata-core.c +10
-0+*/ +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 ;
I would think the proper return value for this case is -ENODEV.
Otherwise OK.
Jeff
PATCH 4/4 libata: revalidate after transfer mode configuration
In reply to Tejun HeoRevalidate 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(-)
05f0163669d505077e04f114027bea39083320bc
drivers/scsi/libata-core.c +6 -0
@@ -1621,6 +1621,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.2.1
Re: [PATCH 4/4] libata: revalidate after transfer mode configuration
Quoted 4 lines · Tejun Heo
Revalidate device after transfer mode configuration. This also makes
dev->idup-to-date.Signed-off-by Tejun Heo <[email protected]>
ACK 1, 2 and 4.