[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Working around bogus HPAs in libata



On Fri, 2010-04-02 at 12:24 +0900, Tejun Heo wrote:
> On 03/22/2010 02:25 AM, Ben Hutchings wrote:
> > Since SCSI has no concept of the Host Protected Area (HPA) supported by
> > ATA, ATA disks handled by libata have no set_capacity() operation and it
> > appears to be impossible to override an HPA except through the libata
> > module parameter.
> > 
> > In particular, this means that the workaround for bogus HPAs in
> > rescan_partitions() does not work:
> > 
> > 			if (bdops->set_capacity &&
> > 			    (disk->flags & GENHD_FL_NATIVE_CAPACITY) == 0) {
> > 				printk(KERN_CONT "enabling native capacity\n");
> > 				capacity = bdops->set_capacity(disk, ~0ULL);
> > 				disk->flags |= GENHD_FL_NATIVE_CAPACITY;
> > 				if (capacity > get_capacity(disk)) {
> > 					set_capacity(disk, capacity);
> > 					check_disk_size_change(disk, bdev);
> > 					bdev->bd_invalidated = 0;
> > 				}
> 
> Hmmm, yeah, this is much better than the dumb switch we have now.  I
> wonder why nobody pointed it out before.

Several distributions have turned on the switch by default.  As I said
previously, this turned out to be a bad idea.

> Does the following patch work as expected?

Without the patch:

[    2.853930] sd 0:0:1:0: [sdb] 150000000 512-byte logical blocks: (76.8 GB/71.5 GiB)
[    2.854220] sd 0:0:1:0: [sdb] Write Protect is off
[    2.856273] sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
[    2.856786]  sdb: sdb1 sdb2 < >
[    2.914459] sdb: p1 size 150866352 exceeds device capacity, limited to end of disk
[    2.914631] sdb: p2 ignored, start 150866415 is behind the end of the disk
[    2.938499] sd 0:0:1:0: [sdb] Attached SCSI disk

With the patch:

[    2.871114] sd 0:0:1:0: [sdb] 150000000 512-byte logical blocks: (76.8 GB/71.5 GiB)
[    2.871395] sd 0:0:1:0: [sdb] Write Protect is off
[    2.871601] sd 0:0:1:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
[    2.872177]  sdb: sdb1 sdb2 < >
[    2.914556] sdb: p1 size 150866352 exceeds device capacity, enabling native capacity
[    2.914753] ata1: soft resetting link
[    3.116372] ata1.01: n_sectors mismatch 150000000 != 156301488
[    3.116433] ata1.01: new n_sectors matches native, probably late HPA unlock, n_sectors adjusted
[    3.164409] ata1.01: configured for UDMA/100
[    3.164471] ata1: EH complete
[    3.164723] sdb: detected capacity change from 76800000000 to 80026361856
[    3.179393] sd 0:0:1:0: [sdb] 156301488 512-byte logical blocks: (80.0 GB/74.5 GiB)
[    3.179842] sd 0:0:1:0: [sdb] Attached SCSI disk
[    3.213602] sdb: detected capacity change from 0 to 80026361856
[    3.214000] sdb: detected capacity change from 0 to 80026361856

For comparison, using an IDE driver:

[    4.172438] hdb: max request size: 128KiB
[    4.173740] hdb: Host Protected Area detected.
[    4.173743] 	current capacity is 150000000 sectors (76800 MB)
[    4.173745] 	native  capacity is 156301488 sectors (80026 MB)
[    4.173922] hdb: 150000000 sectors (76800 MB) w/8192KiB Cache, CHS=65535/16/63
[    4.177393] hdb: cache flushes supported
[    4.177549]  hdb: hdb1 hdb2 < >
[    4.183025] hdb: p1 size 150866352 exceeds device capacity, enabling native capacity
[    4.183842] hdb: detected capacity change from 76800000000 to 80026361856

So, this looks good, but I'm slightly disturbed by the multiple
'capacity change' messages when using libata.  Also, the warnings in
ata_dev_revalidate() should not be shown when we are deliberately
changing capacity.

My test case also revealed a bug in the partition scanning retry:
partition 2 is an extended partition and wholly within the HPA, so the
extended partition table is not accessible until the HPA is disabled.
But it is not rescanned after the HPA is disabled.  However, this is is
consistent between IDE and libata drivers and will not be a regression
when making the transition.

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

Attachment: signature.asc
Description: This is a digitally signed message part


Reply to: