"EXT3-fs error" after resume from s2ram

Submitted by Tejun Heo on July 8, 2009, 3:21 p.m.

Details

Message ID 4A54B98B.50806@gmail.com
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Tejun Heo July 8, 2009, 3:21 p.m.
Tejun Heo wrote:
> One thing we can do is to make libata remember the native size on
> initial probing and let revalidation consider it.  Yeap, that would
> work.  Can you please test the attached patch and post full log
> including boot and suspend/resume?

Patch slightly updated.  Please test this one.

Comments

Christof Warlich July 9, 2009, 6:17 p.m.
Tejun Heo schrieb:
> Tejun Heo wrote:
>   
>> One thing we can do is to make libata remember the native size on
>> initial probing and let revalidation consider it.  Yeap, that would
>> work.  Can you please test the attached patch and post full log
>> including boot and suspend/resume?
>>     
>
> Patch slightly updated.  Please test this one.
>
>   

Hi Tejun, Robert,

hey, great: Suspend to ram seems to work like a charm now! Many thanks 
to Robert for taking the time to identify the root cause and to Tejun 
for providing the patch.

I've attached both boot and suspend / resume messages from 
/var/log/kern.log, just let me know if I can do anything else to
help getting the patch into an official kernel release. Furthermore, I 
would be happy to be kept posted on the progress being made regarding this.

Just one clarification: From what I understood so far, it will be 
impossible to ever utilize the full disk capacity as long as the BIOS 
applies the HPA on boot, right?

Cheers,

Christof
Robert Hancock July 9, 2009, 11:31 p.m.
On Thu, Jul 9, 2009 at 12:17 PM, Christof Warlich<christof@warlich.name> wrote:
> Hi Tejun, Robert,
>
> hey, great: Suspend to ram seems to work like a charm now! Many thanks to
> Robert for taking the time to identify the root cause and to Tejun for
> providing the patch.
>
> I've attached both boot and suspend / resume messages from
> /var/log/kern.log, just let me know if I can do anything else to
> help getting the patch into an official kernel release. Furthermore, I would
> be happy to be kept posted on the progress being made regarding this.
>
> Just one clarification: From what I understood so far, it will be impossible
> to ever utilize the full disk capacity as long as the BIOS applies the HPA
> on boot, right?

I believe that's the case, yes.. if the BIOS is applying the HPA on
each boot, then we can't change the max address again without a
hardware reset..
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tejun Heo July 10, 2009, 1:21 p.m.
Hello,

Robert Hancock wrote:
> On Thu, Jul 9, 2009 at 12:17 PM, Christof Warlich<christof@warlich.name> wrote:
>> Just one clarification: From what I understood so far, it will be impossible
>> to ever utilize the full disk capacity as long as the BIOS applies the HPA
>> on boot, right?
> 
> I believe that's the case, yes.. if the BIOS is applying the HPA on
> each boot, then we can't change the max address again without a
> hardware reset..

This is one of the reasons why libata was switched to prefer hardreset
so that BIOS lock downs can be circumvented.  PATA drivers
unfortunately usually don't implement hardreset.  :-(

I'll forward the patch upstream.

Thanks.

Patch hide | download patch | download mbox

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 045a486..111c5c9 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1515,6 +1515,7 @@  static int ata_hpa_resize(struct ata_device *dev)
 
 		return rc;
 	}
+	dev->n_native_sectors = native_sectors;
 
 	/* nothing to do? */
 	if (native_sectors <= sectors || !ata_ignore_hpa) {
@@ -4089,6 +4090,7 @@  int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
 		       unsigned int readid_flags)
 {
 	u64 n_sectors = dev->n_sectors;
+	u64 n_native_sectors = dev->n_native_sectors;
 	int rc;
 
 	if (!ata_dev_enabled(dev))
@@ -4118,16 +4120,31 @@  int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
 	/* verify n_sectors hasn't changed */
 	if (dev->class == ATA_DEV_ATA && n_sectors &&
 	    dev->n_sectors != n_sectors) {
-		ata_dev_printk(dev, KERN_INFO, "n_sectors mismatch "
+		ata_dev_printk(dev, KERN_WARNING, "n_sectors mismatch "
 			       "%llu != %llu\n",
 			       (unsigned long long)n_sectors,
 			       (unsigned long long)dev->n_sectors);
 
-		/* restore original n_sectors */
-		dev->n_sectors = n_sectors;
-
-		rc = -ENODEV;
-		goto fail;
+		/*
+		 * Something could have caused HPA to be unlocked
+		 * involuntarily.  If n_native_sectors hasn't changed
+		 * and the new size matches it, keep the device.
+		 */
+		if (dev->n_native_sectors == n_native_sectors &&
+		    dev->n_sectors > n_sectors &&
+		    dev->n_sectors == n_native_sectors) {
+			ata_dev_printk(dev, KERN_WARNING,
+				       "new n_sectors matches native, probably "
+				       "late HPA unlock, continuing\n");
+			dev->n_native_sectors = n_native_sectors;
+			dev->n_sectors = n_sectors;
+		} else {
+			/* restore original n_[native]_sectors */
+			dev->n_native_sectors = n_native_sectors;
+			dev->n_sectors = n_sectors;
+			rc = -ENODEV;
+			goto fail;
+		}
 	}
 
 	return 0;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3d501db..5fde0a9 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -588,6 +588,7 @@  struct ata_device {
 #endif
 	/* n_sector is CLEAR_BEGIN, read comment above CLEAR_BEGIN */
 	u64			n_sectors;	/* size of device, if ATA */
+	u64			n_native_sectors; /* native size, if ATA */
 	unsigned int		class;		/* ATA_DEV_xxx */
 	unsigned long		unpark_deadline;