* [PATCHSET] block,libata: implement ->unlock_native_capacity()
@ 2010-05-15 18:09 Tejun Heo
2010-05-15 18:09 ` [PATCH 1/8] buffer: make invalidate_bdev() drain all percpu LRU add caches Tejun Heo
` (9 more replies)
0 siblings, 10 replies; 31+ messages in thread
From: Tejun Heo @ 2010-05-15 18:09 UTC (permalink / raw)
To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley,
linux-kernel, ben, davem
Hello, all.
This is the evolved version of the implement-set_capacity patchset[L]
and contains the following patches.
0001-buffer-make-invalidate_bdev-drain-all-percpu-LRU-add.patch
0002-block-restart-partition-scan-after-resizing-a-device.patch
0003-block-ide-simplify-bdops-set_capacity-to-unlock_nati.patch
0004-block-use-struct-parsed_partitions-state-universally.patch
0005-block-improve-automatic-native-capacity-unlocking.patch
0006-SCSI-implement-sd_unlock_native_capacity.patch
0007-libata-use-the-enlarged-capacity-after-late-HPA-unlo.patch
0008-libata-implement-on-demand-HPA-unlocking.patch
0001-0005 fix and update partition check related code and make the
existing automatic native capacity unlocking mechanism simpler and
more robust.
0001 fixes invalidate_bdev() which seems to have been somewhat broken
for quite some time since percpu LRU add cache was introduced.
0002 makes partition code to restart from beginning after native
capacity is locked so that resize and all other stuff can be dealt
with in the usual revalidation path.
0003 renames ->set_capacity() to ->unlock_native_capacity() and drops
the spurious new capacity parameter and return value.
0004 updates partition check API for the next patch.
0005 improves automatic native capacity unlocking code so that it
covers all partition misdetection cases which can be caused by limited
disk capacity.
0006-0008 add ->unlock_native_capacity() support to libata. 0001-0005
are probably best routed through block tree if David is okay with it
(0003 modifies ide), and 0006-0008 through libata if James is okay
with the SCSI part. What do you guys think?
This patchset is available in the following git branch.
git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata-dev.git unlock-native-capa
diffstat follows.
drivers/ata/libata-core.c | 6 +--
drivers/ata/libata-scsi.c | 29 ++++++++++++++
drivers/ide/ide-disk.c | 40 ++++++++------------
drivers/ide/ide-gd.c | 11 ++---
drivers/scsi/sd.c | 22 +++++++++++
fs/buffer.c | 1
fs/partitions/acorn.c | 68 ++++++++++++++++------------------
fs/partitions/acorn.h | 10 ++---
fs/partitions/amiga.c | 13 +++---
fs/partitions/amiga.h | 2 -
fs/partitions/atari.c | 8 ++--
fs/partitions/atari.h | 2 -
fs/partitions/check.c | 84 ++++++++++++++++++++++++++++++------------
fs/partitions/check.h | 12 ++++++
fs/partitions/efi.c | 91 +++++++++++++++++++++-------------------------
fs/partitions/efi.h | 2 -
fs/partitions/ibm.c | 21 +++++-----
fs/partitions/ibm.h | 2 -
fs/partitions/karma.c | 4 +-
fs/partitions/karma.h | 2 -
fs/partitions/ldm.c | 89 ++++++++++++++++++++++----------------------
fs/partitions/ldm.h | 2 -
fs/partitions/mac.c | 11 +++--
fs/partitions/mac.h | 2 -
fs/partitions/msdos.c | 85 ++++++++++++++++++------------------------
fs/partitions/msdos.h | 2 -
fs/partitions/osf.c | 4 +-
fs/partitions/osf.h | 2 -
fs/partitions/sgi.c | 6 +--
fs/partitions/sgi.h | 2 -
fs/partitions/sun.c | 6 +--
fs/partitions/sun.h | 2 -
fs/partitions/sysv68.c | 6 +--
fs/partitions/sysv68.h | 2 -
fs/partitions/ultrix.c | 4 +-
fs/partitions/ultrix.h | 2 -
include/linux/blkdev.h | 3 -
include/linux/ide.h | 2 -
include/linux/libata.h | 2 +
include/scsi/scsi_host.h | 8 ++++
40 files changed, 375 insertions(+), 297 deletions(-)
Thanks.
--
tejun
[L] http://thread.gmane.org/gmane.linux.scsi/58925
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 1/8] buffer: make invalidate_bdev() drain all percpu LRU add caches 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo @ 2010-05-15 18:09 ` Tejun Heo 2010-05-16 7:11 ` David Miller 2010-05-15 18:09 ` [PATCH 2/8] block: restart partition scan after resizing a device Tejun Heo ` (8 subsequent siblings) 9 siblings, 1 reply; 31+ messages in thread From: Tejun Heo @ 2010-05-15 18:09 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Cc: Tejun Heo invalidate_bdev() should release all page cache pages which are clean and not being used; however, if some pages are still in the percpu LRU add caches on other cpus, those pages are considered in used and don't get released. Fix it by calling lru_add_drain_all() before trying to invalidate pages. This problem was discovered while testing block automatic native capacity unlocking. Null pages which were read before automatic unlocking didn't get released by invalidate_bdev() and ended up interfering with partition scan after unlocking. Signed-off-by: Tejun Heo <tj@kernel.org> --- fs/buffer.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index c9c266d..08e422d 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -275,6 +275,7 @@ void invalidate_bdev(struct block_device *bdev) return; invalidate_bh_lrus(); + lru_add_drain_all(); /* make sure all lru add caches are flushed */ invalidate_mapping_pages(mapping, 0, -1); } EXPORT_SYMBOL(invalidate_bdev); -- 1.6.4.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 1/8] buffer: make invalidate_bdev() drain all percpu LRU add caches 2010-05-15 18:09 ` [PATCH 1/8] buffer: make invalidate_bdev() drain all percpu LRU add caches Tejun Heo @ 2010-05-16 7:11 ` David Miller 0 siblings, 0 replies; 31+ messages in thread From: David Miller @ 2010-05-16 7:11 UTC (permalink / raw) To: tj Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, bzolnier From: Tejun Heo <tj@kernel.org> Date: Sat, 15 May 2010 20:09:27 +0200 > invalidate_bdev() should release all page cache pages which are clean > and not being used; however, if some pages are still in the percpu LRU > add caches on other cpus, those pages are considered in used and don't > get released. Fix it by calling lru_add_drain_all() before trying to > invalidate pages. > > This problem was discovered while testing block automatic native > capacity unlocking. Null pages which were read before automatic > unlocking didn't get released by invalidate_bdev() and ended up > interfering with partition scan after unlocking. > > Signed-off-by: Tejun Heo <tj@kernel.org> Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 2/8] block: restart partition scan after resizing a device 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo 2010-05-15 18:09 ` [PATCH 1/8] buffer: make invalidate_bdev() drain all percpu LRU add caches Tejun Heo @ 2010-05-15 18:09 ` Tejun Heo 2010-05-16 7:15 ` David Miller 2010-05-15 18:09 ` [PATCH 3/8] block,ide: simplify bdops->set_capacity() to ->unlock_native_capacity() Tejun Heo ` (7 subsequent siblings) 9 siblings, 1 reply; 31+ messages in thread From: Tejun Heo @ 2010-05-15 18:09 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Cc: Tejun Heo Device resize via ->set_capacity() can reveal new partitions (e.g. in chained partition table formats such as dos extended parts). Restart partition scan from the beginning after resizing a device. This change also makes libata always revalidate the disk after resize which makes lower layer native capacity unlocking implementation simpler and more robust as resize can be handled in the usual path. Signed-off-by: Tejun Heo <tj@kernel.org> Reported-by: Ben Hutchings <ben@decadent.org.uk> --- fs/partitions/check.c | 16 ++++++---------- 1 files changed, 6 insertions(+), 10 deletions(-) diff --git a/fs/partitions/check.c b/fs/partitions/check.c index e238ab2..8f01df3 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -544,7 +544,7 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev) struct hd_struct *part; struct parsed_partitions *state; int p, highest, res; - +rescan: if (bdev->bd_part_count) return -EBUSY; res = invalidate_partition(disk, 0); @@ -581,7 +581,7 @@ int rescan_partitions(struct gendisk *disk, struct block_device *bdev) /* add partitions */ for (p = 1; p < state->limit; p++) { sector_t size, from; -try_scan: + size = state->parts[p].size; if (!size) continue; @@ -596,7 +596,6 @@ try_scan: if (from + size > get_capacity(disk)) { const struct block_device_operations *bdops = disk->fops; - unsigned long long capacity; printk(KERN_WARNING "%s: p%d size %llu exceeds device capacity, ", @@ -605,14 +604,11 @@ try_scan: if (bdops->set_capacity && (disk->flags & GENHD_FL_NATIVE_CAPACITY) == 0) { printk(KERN_CONT "enabling native capacity\n"); - capacity = bdops->set_capacity(disk, ~0ULL); + 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; - } - goto try_scan; + /* free state and restart */ + kfree(state); + goto rescan; } else { /* * we can not ignore partitions of broken tables -- 1.6.4.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 2/8] block: restart partition scan after resizing a device 2010-05-15 18:09 ` [PATCH 2/8] block: restart partition scan after resizing a device Tejun Heo @ 2010-05-16 7:15 ` David Miller 0 siblings, 0 replies; 31+ messages in thread From: David Miller @ 2010-05-16 7:15 UTC (permalink / raw) To: tj Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, bzolnier From: Tejun Heo <tj@kernel.org> Date: Sat, 15 May 2010 20:09:28 +0200 > Device resize via ->set_capacity() can reveal new partitions (e.g. in > chained partition table formats such as dos extended parts). Restart > partition scan from the beginning after resizing a device. This > change also makes libata always revalidate the disk after resize which > makes lower layer native capacity unlocking implementation simpler and > more robust as resize can be handled in the usual path. > > Signed-off-by: Tejun Heo <tj@kernel.org> > Reported-by: Ben Hutchings <ben@decadent.org.uk> Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/8] block,ide: simplify bdops->set_capacity() to ->unlock_native_capacity() 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo 2010-05-15 18:09 ` [PATCH 1/8] buffer: make invalidate_bdev() drain all percpu LRU add caches Tejun Heo 2010-05-15 18:09 ` [PATCH 2/8] block: restart partition scan after resizing a device Tejun Heo @ 2010-05-15 18:09 ` Tejun Heo 2010-05-15 18:48 ` Bartlomiej Zolnierkiewicz 2010-05-16 7:15 ` David Miller 2010-05-15 18:09 ` [PATCH 4/8] block: use struct parsed_partitions *state universally in partition check code Tejun Heo ` (6 subsequent siblings) 9 siblings, 2 replies; 31+ messages in thread From: Tejun Heo @ 2010-05-15 18:09 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Cc: Tejun Heo bdops->set_capacity() is unnecessarily generic. All that's required is a simple one way notification to lower level driver telling it to try to unlock native capacity. There's no reason to pass in target capacity or return the new capacity. The former is always the inherent native capacity and the latter can be handled via the usual device resize / revalidation path. In fact, the current API is always used that way. Replace ->set_capacity() with ->unlock_native_capacity() which take only @disk and doesn't return anything. IDE which is the only current user of the API is converted accordingly. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Ben Hutchings <ben@decadent.org.uk> Cc: Jens Axboe <jens.axboe@oracle.com> Cc: David Miller <davem@davemloft.net> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> --- drivers/ide/ide-disk.c | 40 ++++++++++++++++------------------------ drivers/ide/ide-gd.c | 11 ++++------- fs/partitions/check.c | 4 ++-- include/linux/blkdev.h | 3 +-- include/linux/ide.h | 2 +- 5 files changed, 24 insertions(+), 36 deletions(-) diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c index 3b128dc..33d6503 100644 --- a/drivers/ide/ide-disk.c +++ b/drivers/ide/ide-disk.c @@ -407,32 +407,24 @@ static int ide_disk_get_capacity(ide_drive_t *drive) return 0; } -static u64 ide_disk_set_capacity(ide_drive_t *drive, u64 capacity) +static void ide_disk_unlock_native_capacity(ide_drive_t *drive) { - u64 set = min(capacity, drive->probed_capacity); u16 *id = drive->id; int lba48 = ata_id_lba48_enabled(id); if ((drive->dev_flags & IDE_DFLAG_LBA) == 0 || ata_id_hpa_enabled(id) == 0) - goto out; + return; /* * according to the spec the SET MAX ADDRESS command shall be * immediately preceded by a READ NATIVE MAX ADDRESS command */ - capacity = ide_disk_hpa_get_native_capacity(drive, lba48); - if (capacity == 0) - goto out; - - set = ide_disk_hpa_set_capacity(drive, set, lba48); - if (set) { - /* needed for ->resume to disable HPA */ - drive->dev_flags |= IDE_DFLAG_NOHPA; - return set; - } -out: - return drive->capacity64; + if (!ide_disk_hpa_get_native_capacity(drive, lba48)) + return; + + if (ide_disk_hpa_set_capacity(drive, drive->probed_capacity, lba48)) + drive->dev_flags |= IDE_DFLAG_NOHPA; /* disable HPA on resume */ } static void idedisk_prepare_flush(struct request_queue *q, struct request *rq) @@ -783,13 +775,13 @@ static int ide_disk_set_doorlock(ide_drive_t *drive, struct gendisk *disk, } const struct ide_disk_ops ide_ata_disk_ops = { - .check = ide_disk_check, - .set_capacity = ide_disk_set_capacity, - .get_capacity = ide_disk_get_capacity, - .setup = ide_disk_setup, - .flush = ide_disk_flush, - .init_media = ide_disk_init_media, - .set_doorlock = ide_disk_set_doorlock, - .do_request = ide_do_rw_disk, - .ioctl = ide_disk_ioctl, + .check = ide_disk_check, + .unlock_native_capacity = ide_disk_unlock_native_capacity, + .get_capacity = ide_disk_get_capacity, + .setup = ide_disk_setup, + .flush = ide_disk_flush, + .init_media = ide_disk_init_media, + .set_doorlock = ide_disk_set_doorlock, + .do_request = ide_do_rw_disk, + .ioctl = ide_disk_ioctl, }; diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c index c32d839..c102d23 100644 --- a/drivers/ide/ide-gd.c +++ b/drivers/ide/ide-gd.c @@ -288,17 +288,14 @@ static int ide_gd_media_changed(struct gendisk *disk) return ret; } -static unsigned long long ide_gd_set_capacity(struct gendisk *disk, - unsigned long long capacity) +static void ide_gd_unlock_native_capacity(struct gendisk *disk) { struct ide_disk_obj *idkp = ide_drv_g(disk, ide_disk_obj); ide_drive_t *drive = idkp->drive; const struct ide_disk_ops *disk_ops = drive->disk_ops; - if (disk_ops->set_capacity) - return disk_ops->set_capacity(drive, capacity); - - return drive->capacity64; + if (disk_ops->unlock_native_capacity) + disk_ops->unlock_native_capacity(drive); } static int ide_gd_revalidate_disk(struct gendisk *disk) @@ -329,7 +326,7 @@ static const struct block_device_operations ide_gd_ops = { .locked_ioctl = ide_gd_ioctl, .getgeo = ide_gd_getgeo, .media_changed = ide_gd_media_changed, - .set_capacity = ide_gd_set_capacity, + .unlock_native_capacity = ide_gd_unlock_native_capacity, .revalidate_disk = ide_gd_revalidate_disk }; diff --git a/fs/partitions/check.c b/fs/partitions/check.c index 8f01df3..4f1fee0 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -601,10 +601,10 @@ rescan: "%s: p%d size %llu exceeds device capacity, ", disk->disk_name, p, (unsigned long long) size); - if (bdops->set_capacity && + if (bdops->unlock_native_capacity && (disk->flags & GENHD_FL_NATIVE_CAPACITY) == 0) { printk(KERN_CONT "enabling native capacity\n"); - bdops->set_capacity(disk, ~0ULL); + bdops->unlock_native_capacity(disk); disk->flags |= GENHD_FL_NATIVE_CAPACITY; /* free state and restart */ kfree(state); diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 6690e8b..f2a0c33 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1283,8 +1283,7 @@ struct block_device_operations { int (*direct_access) (struct block_device *, sector_t, void **, unsigned long *); int (*media_changed) (struct gendisk *); - unsigned long long (*set_capacity) (struct gendisk *, - unsigned long long); + void (*unlock_native_capacity) (struct gendisk *); int (*revalidate_disk) (struct gendisk *); int (*getgeo)(struct block_device *, struct hd_geometry *); struct module *owner; diff --git a/include/linux/ide.h b/include/linux/ide.h index 3239d1c..b6d4480 100644 --- a/include/linux/ide.h +++ b/include/linux/ide.h @@ -362,7 +362,7 @@ struct ide_drive_s; struct ide_disk_ops { int (*check)(struct ide_drive_s *, const char *); int (*get_capacity)(struct ide_drive_s *); - u64 (*set_capacity)(struct ide_drive_s *, u64); + void (*unlock_native_capacity)(struct ide_drive_s *); void (*setup)(struct ide_drive_s *); void (*flush)(struct ide_drive_s *); int (*init_media)(struct ide_drive_s *, struct gendisk *); -- 1.6.4.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 3/8] block,ide: simplify bdops->set_capacity() to ->unlock_native_capacity() 2010-05-15 18:09 ` [PATCH 3/8] block,ide: simplify bdops->set_capacity() to ->unlock_native_capacity() Tejun Heo @ 2010-05-15 18:48 ` Bartlomiej Zolnierkiewicz 2010-05-15 18:58 ` Tejun Heo 2010-05-16 7:15 ` David Miller 1 sibling, 1 reply; 31+ messages in thread From: Bartlomiej Zolnierkiewicz @ 2010-05-15 18:48 UTC (permalink / raw) To: Tejun Heo Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Hi Tejun, On Saturday 15 May 2010 08:09:29 pm Tejun Heo wrote: > bdops->set_capacity() is unnecessarily generic. All that's required > is a simple one way notification to lower level driver telling it to > try to unlock native capacity. There's no reason to pass in target > capacity or return the new capacity. The former is always the This seems to rely on an optimistic assumption that command execution will always be successful and that no errors on disk or host level can ever happen.. [ The assumption in question was introduced in the previous patch: @@ -605,14 +604,11 @@ try_scan: if (bdops->set_capacity && (disk->flags & GENHD_FL_NATIVE_CAPACITY) == 0) { printk(KERN_CONT "enabling native capacity\n"); - capacity = bdops->set_capacity(disk, ~0ULL); + 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; - } - goto try_scan; + /* free state and restart */ + kfree(state); + goto rescan; originally if the command execution failed the 'capacity' would be 0 ] It also seems that the original code could be improved a lot to handle (very unlikely but not impossible) error situations better.. > inherent native capacity and the latter can be handled via the usual > device resize / revalidation path. In fact, the current API is always > used that way. > > Replace ->set_capacity() with ->unlock_native_capacity() which take > only @disk and doesn't return anything. IDE which is the only current > user of the API is converted accordingly. > > Signed-off-by: Tejun Heo <tj@kernel.org> > Cc: Ben Hutchings <ben@decadent.org.uk> > Cc: Jens Axboe <jens.axboe@oracle.com> > Cc: David Miller <davem@davemloft.net> > Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> > --- > drivers/ide/ide-disk.c | 40 ++++++++++++++++------------------------ > drivers/ide/ide-gd.c | 11 ++++------- > fs/partitions/check.c | 4 ++-- > include/linux/blkdev.h | 3 +-- > include/linux/ide.h | 2 +- > 5 files changed, 24 insertions(+), 36 deletions(-) > > diff --git a/drivers/ide/ide-disk.c b/drivers/ide/ide-disk.c > index 3b128dc..33d6503 100644 > --- a/drivers/ide/ide-disk.c > +++ b/drivers/ide/ide-disk.c > @@ -407,32 +407,24 @@ static int ide_disk_get_capacity(ide_drive_t *drive) > return 0; > } > > -static u64 ide_disk_set_capacity(ide_drive_t *drive, u64 capacity) > +static void ide_disk_unlock_native_capacity(ide_drive_t *drive) > { > - u64 set = min(capacity, drive->probed_capacity); > u16 *id = drive->id; > int lba48 = ata_id_lba48_enabled(id); > > if ((drive->dev_flags & IDE_DFLAG_LBA) == 0 || > ata_id_hpa_enabled(id) == 0) > - goto out; > + return; > > /* > * according to the spec the SET MAX ADDRESS command shall be > * immediately preceded by a READ NATIVE MAX ADDRESS command > */ > - capacity = ide_disk_hpa_get_native_capacity(drive, lba48); > - if (capacity == 0) > - goto out; > - > - set = ide_disk_hpa_set_capacity(drive, set, lba48); > - if (set) { > - /* needed for ->resume to disable HPA */ > - drive->dev_flags |= IDE_DFLAG_NOHPA; > - return set; > - } > -out: > - return drive->capacity64; > + if (!ide_disk_hpa_get_native_capacity(drive, lba48)) > + return; > + > + if (ide_disk_hpa_set_capacity(drive, drive->probed_capacity, lba48)) > + drive->dev_flags |= IDE_DFLAG_NOHPA; /* disable HPA on resume */ > } > > static void idedisk_prepare_flush(struct request_queue *q, struct request *rq) > @@ -783,13 +775,13 @@ static int ide_disk_set_doorlock(ide_drive_t *drive, struct gendisk *disk, > } > > const struct ide_disk_ops ide_ata_disk_ops = { > - .check = ide_disk_check, > - .set_capacity = ide_disk_set_capacity, > - .get_capacity = ide_disk_get_capacity, > - .setup = ide_disk_setup, > - .flush = ide_disk_flush, > - .init_media = ide_disk_init_media, > - .set_doorlock = ide_disk_set_doorlock, > - .do_request = ide_do_rw_disk, > - .ioctl = ide_disk_ioctl, > + .check = ide_disk_check, > + .unlock_native_capacity = ide_disk_unlock_native_capacity, > + .get_capacity = ide_disk_get_capacity, > + .setup = ide_disk_setup, > + .flush = ide_disk_flush, > + .init_media = ide_disk_init_media, > + .set_doorlock = ide_disk_set_doorlock, > + .do_request = ide_do_rw_disk, > + .ioctl = ide_disk_ioctl, > }; > diff --git a/drivers/ide/ide-gd.c b/drivers/ide/ide-gd.c > index c32d839..c102d23 100644 > --- a/drivers/ide/ide-gd.c > +++ b/drivers/ide/ide-gd.c > @@ -288,17 +288,14 @@ static int ide_gd_media_changed(struct gendisk *disk) > return ret; > } > > -static unsigned long long ide_gd_set_capacity(struct gendisk *disk, > - unsigned long long capacity) > +static void ide_gd_unlock_native_capacity(struct gendisk *disk) > { > struct ide_disk_obj *idkp = ide_drv_g(disk, ide_disk_obj); > ide_drive_t *drive = idkp->drive; > const struct ide_disk_ops *disk_ops = drive->disk_ops; > > - if (disk_ops->set_capacity) > - return disk_ops->set_capacity(drive, capacity); > - > - return drive->capacity64; > + if (disk_ops->unlock_native_capacity) > + disk_ops->unlock_native_capacity(drive); > } > > static int ide_gd_revalidate_disk(struct gendisk *disk) > @@ -329,7 +326,7 @@ static const struct block_device_operations ide_gd_ops = { > .locked_ioctl = ide_gd_ioctl, > .getgeo = ide_gd_getgeo, > .media_changed = ide_gd_media_changed, > - .set_capacity = ide_gd_set_capacity, > + .unlock_native_capacity = ide_gd_unlock_native_capacity, > .revalidate_disk = ide_gd_revalidate_disk > }; > > diff --git a/fs/partitions/check.c b/fs/partitions/check.c > index 8f01df3..4f1fee0 100644 > --- a/fs/partitions/check.c > +++ b/fs/partitions/check.c > @@ -601,10 +601,10 @@ rescan: > "%s: p%d size %llu exceeds device capacity, ", > disk->disk_name, p, (unsigned long long) size); > > - if (bdops->set_capacity && > + if (bdops->unlock_native_capacity && > (disk->flags & GENHD_FL_NATIVE_CAPACITY) == 0) { > printk(KERN_CONT "enabling native capacity\n"); > - bdops->set_capacity(disk, ~0ULL); > + bdops->unlock_native_capacity(disk); > disk->flags |= GENHD_FL_NATIVE_CAPACITY; > /* free state and restart */ > kfree(state); > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index 6690e8b..f2a0c33 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -1283,8 +1283,7 @@ struct block_device_operations { > int (*direct_access) (struct block_device *, sector_t, > void **, unsigned long *); > int (*media_changed) (struct gendisk *); > - unsigned long long (*set_capacity) (struct gendisk *, > - unsigned long long); > + void (*unlock_native_capacity) (struct gendisk *); > int (*revalidate_disk) (struct gendisk *); > int (*getgeo)(struct block_device *, struct hd_geometry *); > struct module *owner; > diff --git a/include/linux/ide.h b/include/linux/ide.h > index 3239d1c..b6d4480 100644 > --- a/include/linux/ide.h > +++ b/include/linux/ide.h > @@ -362,7 +362,7 @@ struct ide_drive_s; > struct ide_disk_ops { > int (*check)(struct ide_drive_s *, const char *); > int (*get_capacity)(struct ide_drive_s *); > - u64 (*set_capacity)(struct ide_drive_s *, u64); > + void (*unlock_native_capacity)(struct ide_drive_s *); > void (*setup)(struct ide_drive_s *); > void (*flush)(struct ide_drive_s *); > int (*init_media)(struct ide_drive_s *, struct gendisk *); ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/8] block,ide: simplify bdops->set_capacity() to ->unlock_native_capacity() 2010-05-15 18:48 ` Bartlomiej Zolnierkiewicz @ 2010-05-15 18:58 ` Tejun Heo 0 siblings, 0 replies; 31+ messages in thread From: Tejun Heo @ 2010-05-15 18:58 UTC (permalink / raw) To: Bartlomiej Zolnierkiewicz Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Hello, Bartlomiej. On 05/15/2010 08:48 PM, Bartlomiej Zolnierkiewicz wrote: > This seems to rely on an optimistic assumption that command execution > will always be successful and that no errors on disk or host level can > ever happen.. > > [ The assumption in question was introduced in the previous patch: ... > originally if the command execution failed the 'capacity' would be 0 ] Hmm... nothing really changes by this tho. Whether unlocking succeeds or not, the block layer will revalidate the disk and the state at that point will be taken as the configuration to use. There really is nothing much else to do. You try unlocking, if it unlocks, use new capacity. If unlocking fails, revalidation will report the limited size and block layer will have to use that. If the device dies due to the unlocking attempt, well, the device is dead all the same. > It also seems that the original code could be improved a lot to handle > (very unlikely but not impossible) error situations better.. If retry is deemed necessary, it's best handled inside the per-driver unlock handler. I don't think pushing EH logic upto block layer in this case would be a good idea. Block layer is basically telling the driver "do whatever you can do to unlock the native capacity, when you're done, I'll restart from the beginning". Thanks. -- tejun ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/8] block,ide: simplify bdops->set_capacity() to ->unlock_native_capacity() 2010-05-15 18:09 ` [PATCH 3/8] block,ide: simplify bdops->set_capacity() to ->unlock_native_capacity() Tejun Heo 2010-05-15 18:48 ` Bartlomiej Zolnierkiewicz @ 2010-05-16 7:15 ` David Miller 1 sibling, 0 replies; 31+ messages in thread From: David Miller @ 2010-05-16 7:15 UTC (permalink / raw) To: tj Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, bzolnier From: Tejun Heo <tj@kernel.org> Date: Sat, 15 May 2010 20:09:29 +0200 > bdops->set_capacity() is unnecessarily generic. All that's required > is a simple one way notification to lower level driver telling it to > try to unlock native capacity. There's no reason to pass in target > capacity or return the new capacity. The former is always the > inherent native capacity and the latter can be handled via the usual > device resize / revalidation path. In fact, the current API is always > used that way. > > Replace ->set_capacity() with ->unlock_native_capacity() which take > only @disk and doesn't return anything. IDE which is the only current > user of the API is converted accordingly. > > Signed-off-by: Tejun Heo <tj@kernel.org> Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 4/8] block: use struct parsed_partitions *state universally in partition check code 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo ` (2 preceding siblings ...) 2010-05-15 18:09 ` [PATCH 3/8] block,ide: simplify bdops->set_capacity() to ->unlock_native_capacity() Tejun Heo @ 2010-05-15 18:09 ` Tejun Heo 2010-05-16 7:16 ` David Miller 2010-05-15 18:09 ` [PATCH 5/8] block: improve automatic native capacity unlocking Tejun Heo ` (5 subsequent siblings) 9 siblings, 1 reply; 31+ messages in thread From: Tejun Heo @ 2010-05-15 18:09 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Cc: Tejun Heo Make the following changes to partition check code. * Add ->bdev to struct parsed_partitions. * Introduce read_part_sector() which is a simple wrapper around read_dev_sector() which takes struct parsed_partitions *state instead of @bdev. * For functions which used to take @state and @bdev, drop @bdev. For functions which used to take @bdev, replace it with @state. * While updating, drop superflous checks on NULL state/bdev in ldm.c. This cleans up the API a bit and enables better handling of IO errors during partition check as the generic partition check code now has much better visibility into what went wrong in the low level code paths. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Ben Hutchings <ben@decadent.org.uk> --- fs/partitions/acorn.c | 68 +++++++++++++++++------------------- fs/partitions/acorn.h | 10 +++--- fs/partitions/amiga.c | 13 +++---- fs/partitions/amiga.h | 2 +- fs/partitions/atari.c | 8 ++-- fs/partitions/atari.h | 2 +- fs/partitions/check.c | 5 ++- fs/partitions/check.h | 7 ++++ fs/partitions/efi.c | 91 ++++++++++++++++++++++------------------------- fs/partitions/efi.h | 2 +- fs/partitions/ibm.c | 21 +++++------ fs/partitions/ibm.h | 2 +- fs/partitions/karma.c | 4 +- fs/partitions/karma.h | 2 +- fs/partitions/ldm.c | 89 +++++++++++++++++++++++----------------------- fs/partitions/ldm.h | 2 +- fs/partitions/mac.c | 11 +++--- fs/partitions/mac.h | 2 +- fs/partitions/msdos.c | 85 +++++++++++++++++++------------------------- fs/partitions/msdos.h | 2 +- fs/partitions/osf.c | 4 +- fs/partitions/osf.h | 2 +- fs/partitions/sgi.c | 6 ++-- fs/partitions/sgi.h | 2 +- fs/partitions/sun.c | 6 ++-- fs/partitions/sun.h | 2 +- fs/partitions/sysv68.c | 6 ++-- fs/partitions/sysv68.h | 2 +- fs/partitions/ultrix.c | 4 +- fs/partitions/ultrix.h | 2 +- 30 files changed, 225 insertions(+), 239 deletions(-) diff --git a/fs/partitions/acorn.c b/fs/partitions/acorn.c index a97b477..6921e78 100644 --- a/fs/partitions/acorn.c +++ b/fs/partitions/acorn.c @@ -70,14 +70,14 @@ struct riscix_record { #if defined(CONFIG_ACORN_PARTITION_CUMANA) || \ defined(CONFIG_ACORN_PARTITION_ADFS) -static int -riscix_partition(struct parsed_partitions *state, struct block_device *bdev, - unsigned long first_sect, int slot, unsigned long nr_sects) +static int riscix_partition(struct parsed_partitions *state, + unsigned long first_sect, int slot, + unsigned long nr_sects) { Sector sect; struct riscix_record *rr; - rr = (struct riscix_record *)read_dev_sector(bdev, first_sect, §); + rr = read_part_sector(state, first_sect, §); if (!rr) return -1; @@ -123,9 +123,9 @@ struct linux_part { #if defined(CONFIG_ACORN_PARTITION_CUMANA) || \ defined(CONFIG_ACORN_PARTITION_ADFS) -static int -linux_partition(struct parsed_partitions *state, struct block_device *bdev, - unsigned long first_sect, int slot, unsigned long nr_sects) +static int linux_partition(struct parsed_partitions *state, + unsigned long first_sect, int slot, + unsigned long nr_sects) { Sector sect; struct linux_part *linuxp; @@ -135,7 +135,7 @@ linux_partition(struct parsed_partitions *state, struct block_device *bdev, put_partition(state, slot++, first_sect, size); - linuxp = (struct linux_part *)read_dev_sector(bdev, first_sect, §); + linuxp = read_part_sector(state, first_sect, §); if (!linuxp) return -1; @@ -157,8 +157,7 @@ linux_partition(struct parsed_partitions *state, struct block_device *bdev, #endif #ifdef CONFIG_ACORN_PARTITION_CUMANA -int -adfspart_check_CUMANA(struct parsed_partitions *state, struct block_device *bdev) +int adfspart_check_CUMANA(struct parsed_partitions *state) { unsigned long first_sector = 0; unsigned int start_blk = 0; @@ -185,7 +184,7 @@ adfspart_check_CUMANA(struct parsed_partitions *state, struct block_device *bdev struct adfs_discrecord *dr; unsigned int nr_sects; - data = read_dev_sector(bdev, start_blk * 2 + 6, §); + data = read_part_sector(state, start_blk * 2 + 6, §); if (!data) return -1; @@ -217,14 +216,14 @@ adfspart_check_CUMANA(struct parsed_partitions *state, struct block_device *bdev #ifdef CONFIG_ACORN_PARTITION_RISCIX case PARTITION_RISCIX_SCSI: /* RISCiX - we don't know how to find the next one. */ - slot = riscix_partition(state, bdev, first_sector, - slot, nr_sects); + slot = riscix_partition(state, first_sector, slot, + nr_sects); break; #endif case PARTITION_LINUX: - slot = linux_partition(state, bdev, first_sector, - slot, nr_sects); + slot = linux_partition(state, first_sector, slot, + nr_sects); break; } put_dev_sector(sect); @@ -249,8 +248,7 @@ adfspart_check_CUMANA(struct parsed_partitions *state, struct block_device *bdev * hda1 = ADFS partition on first drive. * hda2 = non-ADFS partition. */ -int -adfspart_check_ADFS(struct parsed_partitions *state, struct block_device *bdev) +int adfspart_check_ADFS(struct parsed_partitions *state) { unsigned long start_sect, nr_sects, sectscyl, heads; Sector sect; @@ -259,7 +257,7 @@ adfspart_check_ADFS(struct parsed_partitions *state, struct block_device *bdev) unsigned char id; int slot = 1; - data = read_dev_sector(bdev, 6, §); + data = read_part_sector(state, 6, §); if (!data) return -1; @@ -278,21 +276,21 @@ adfspart_check_ADFS(struct parsed_partitions *state, struct block_device *bdev) /* * Work out start of non-adfs partition. */ - nr_sects = (bdev->bd_inode->i_size >> 9) - start_sect; + nr_sects = (state->bdev->bd_inode->i_size >> 9) - start_sect; if (start_sect) { switch (id) { #ifdef CONFIG_ACORN_PARTITION_RISCIX case PARTITION_RISCIX_SCSI: case PARTITION_RISCIX_MFM: - slot = riscix_partition(state, bdev, start_sect, - slot, nr_sects); + slot = riscix_partition(state, start_sect, slot, + nr_sects); break; #endif case PARTITION_LINUX: - slot = linux_partition(state, bdev, start_sect, - slot, nr_sects); + slot = linux_partition(state, start_sect, slot, + nr_sects); break; } } @@ -308,10 +306,11 @@ struct ics_part { __le32 size; }; -static int adfspart_check_ICSLinux(struct block_device *bdev, unsigned long block) +static int adfspart_check_ICSLinux(struct parsed_partitions *state, + unsigned long block) { Sector sect; - unsigned char *data = read_dev_sector(bdev, block, §); + unsigned char *data = read_part_sector(state, block, §); int result = 0; if (data) { @@ -349,8 +348,7 @@ static inline int valid_ics_sector(const unsigned char *data) * hda2 = ADFS partition 1 on first drive. * ..etc.. */ -int -adfspart_check_ICS(struct parsed_partitions *state, struct block_device *bdev) +int adfspart_check_ICS(struct parsed_partitions *state) { const unsigned char *data; const struct ics_part *p; @@ -360,7 +358,7 @@ adfspart_check_ICS(struct parsed_partitions *state, struct block_device *bdev) /* * Try ICS style partitions - sector 0 contains partition info. */ - data = read_dev_sector(bdev, 0, §); + data = read_part_sector(state, 0, §); if (!data) return -1; @@ -392,7 +390,7 @@ adfspart_check_ICS(struct parsed_partitions *state, struct block_device *bdev) * partition is. We must not make this visible * to the filesystem. */ - if (size > 1 && adfspart_check_ICSLinux(bdev, start)) { + if (size > 1 && adfspart_check_ICSLinux(state, start)) { start += 1; size -= 1; } @@ -446,8 +444,7 @@ static inline int valid_ptec_sector(const unsigned char *data) * hda2 = ADFS partition 1 on first drive. * ..etc.. */ -int -adfspart_check_POWERTEC(struct parsed_partitions *state, struct block_device *bdev) +int adfspart_check_POWERTEC(struct parsed_partitions *state) { Sector sect; const unsigned char *data; @@ -455,7 +452,7 @@ adfspart_check_POWERTEC(struct parsed_partitions *state, struct block_device *bd int slot = 1; int i; - data = read_dev_sector(bdev, 0, §); + data = read_part_sector(state, 0, §); if (!data) return -1; @@ -508,8 +505,7 @@ static const char eesox_name[] = { * 1. The individual ADFS boot block entries that are placed on the disk. * 2. The start address of the next entry. */ -int -adfspart_check_EESOX(struct parsed_partitions *state, struct block_device *bdev) +int adfspart_check_EESOX(struct parsed_partitions *state) { Sector sect; const unsigned char *data; @@ -518,7 +514,7 @@ adfspart_check_EESOX(struct parsed_partitions *state, struct block_device *bdev) sector_t start = 0; int i, slot = 1; - data = read_dev_sector(bdev, 7, §); + data = read_part_sector(state, 7, §); if (!data) return -1; @@ -545,7 +541,7 @@ adfspart_check_EESOX(struct parsed_partitions *state, struct block_device *bdev) if (i != 0) { sector_t size; - size = get_capacity(bdev->bd_disk); + size = get_capacity(state->bdev->bd_disk); put_partition(state, slot++, start, size - start); printk("\n"); } diff --git a/fs/partitions/acorn.h b/fs/partitions/acorn.h index 81fd50e..ede8285 100644 --- a/fs/partitions/acorn.h +++ b/fs/partitions/acorn.h @@ -7,8 +7,8 @@ * format, and everyone stick to it? */ -int adfspart_check_CUMANA(struct parsed_partitions *state, struct block_device *bdev); -int adfspart_check_ADFS(struct parsed_partitions *state, struct block_device *bdev); -int adfspart_check_ICS(struct parsed_partitions *state, struct block_device *bdev); -int adfspart_check_POWERTEC(struct parsed_partitions *state, struct block_device *bdev); -int adfspart_check_EESOX(struct parsed_partitions *state, struct block_device *bdev); +int adfspart_check_CUMANA(struct parsed_partitions *state); +int adfspart_check_ADFS(struct parsed_partitions *state); +int adfspart_check_ICS(struct parsed_partitions *state); +int adfspart_check_POWERTEC(struct parsed_partitions *state); +int adfspart_check_EESOX(struct parsed_partitions *state); diff --git a/fs/partitions/amiga.c b/fs/partitions/amiga.c index 9917a8c..ba443d4 100644 --- a/fs/partitions/amiga.c +++ b/fs/partitions/amiga.c @@ -23,8 +23,7 @@ checksum_block(__be32 *m, int size) return sum; } -int -amiga_partition(struct parsed_partitions *state, struct block_device *bdev) +int amiga_partition(struct parsed_partitions *state) { Sector sect; unsigned char *data; @@ -38,11 +37,11 @@ amiga_partition(struct parsed_partitions *state, struct block_device *bdev) for (blk = 0; ; blk++, put_dev_sector(sect)) { if (blk == RDB_ALLOCATION_LIMIT) goto rdb_done; - data = read_dev_sector(bdev, blk, §); + data = read_part_sector(state, blk, §); if (!data) { if (warn_no_part) printk("Dev %s: unable to read RDB block %d\n", - bdevname(bdev, b), blk); + bdevname(state->bdev, b), blk); res = -1; goto rdb_done; } @@ -64,7 +63,7 @@ amiga_partition(struct parsed_partitions *state, struct block_device *bdev) } printk("Dev %s: RDB in block %d has bad checksum\n", - bdevname(bdev, b), blk); + bdevname(state->bdev, b), blk); } /* blksize is blocks per 512 byte standard block */ @@ -75,11 +74,11 @@ amiga_partition(struct parsed_partitions *state, struct block_device *bdev) put_dev_sector(sect); for (part = 1; blk>0 && part<=16; part++, put_dev_sector(sect)) { blk *= blksize; /* Read in terms partition table understands */ - data = read_dev_sector(bdev, blk, §); + data = read_part_sector(state, blk, §); if (!data) { if (warn_no_part) printk("Dev %s: unable to read partition block %d\n", - bdevname(bdev, b), blk); + bdevname(state->bdev, b), blk); res = -1; goto rdb_done; } diff --git a/fs/partitions/amiga.h b/fs/partitions/amiga.h index 2f3e9ce..d094585 100644 --- a/fs/partitions/amiga.h +++ b/fs/partitions/amiga.h @@ -2,5 +2,5 @@ * fs/partitions/amiga.h */ -int amiga_partition(struct parsed_partitions *state, struct block_device *bdev); +int amiga_partition(struct parsed_partitions *state); diff --git a/fs/partitions/atari.c b/fs/partitions/atari.c index 1f3572d..4439ff1 100644 --- a/fs/partitions/atari.c +++ b/fs/partitions/atari.c @@ -30,7 +30,7 @@ static inline int OK_id(char *s) memcmp (s, "RAW", 3) == 0 ; } -int atari_partition(struct parsed_partitions *state, struct block_device *bdev) +int atari_partition(struct parsed_partitions *state) { Sector sect; struct rootsector *rs; @@ -42,12 +42,12 @@ int atari_partition(struct parsed_partitions *state, struct block_device *bdev) int part_fmt = 0; /* 0:unknown, 1:AHDI, 2:ICD/Supra */ #endif - rs = (struct rootsector *) read_dev_sector(bdev, 0, §); + rs = read_part_sector(state, 0, §); if (!rs) return -1; /* Verify this is an Atari rootsector: */ - hd_size = bdev->bd_inode->i_size >> 9; + hd_size = state->bdev->bd_inode->i_size >> 9; if (!VALID_PARTITION(&rs->part[0], hd_size) && !VALID_PARTITION(&rs->part[1], hd_size) && !VALID_PARTITION(&rs->part[2], hd_size) && @@ -84,7 +84,7 @@ int atari_partition(struct parsed_partitions *state, struct block_device *bdev) printk(" XGM<"); partsect = extensect = be32_to_cpu(pi->st); while (1) { - xrs = (struct rootsector *)read_dev_sector(bdev, partsect, §2); + xrs = read_part_sector(state, partsect, §2); if (!xrs) { printk (" block %ld read failed\n", partsect); put_dev_sector(sect); diff --git a/fs/partitions/atari.h b/fs/partitions/atari.h index 63186b0..fe2d32a 100644 --- a/fs/partitions/atari.h +++ b/fs/partitions/atari.h @@ -31,4 +31,4 @@ struct rootsector u16 checksum; /* checksum for bootable disks */ } __attribute__((__packed__)); -int atari_partition(struct parsed_partitions *state, struct block_device *bdev); +int atari_partition(struct parsed_partitions *state); diff --git a/fs/partitions/check.c b/fs/partitions/check.c index 4f1fee0..a19995c 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -45,7 +45,7 @@ extern void md_autodetect_dev(dev_t dev); int warn_no_part = 1; /*This is ugly: should make genhd removable media aware*/ -static int (*check_part[])(struct parsed_partitions *, struct block_device *) = { +static int (*check_part[])(struct parsed_partitions *) = { /* * Probe partition formats with tables at disk address 0 * that also have an ADFS boot block at 0xdc0. @@ -165,6 +165,7 @@ check_partition(struct gendisk *hd, struct block_device *bdev) if (!state) return NULL; + state->bdev = bdev; disk_name(hd, 0, state->name); printk(KERN_INFO " %s:", state->name); if (isdigit(state->name[strlen(state->name)-1])) @@ -174,7 +175,7 @@ check_partition(struct gendisk *hd, struct block_device *bdev) i = res = err = 0; while (!res && check_part[i]) { memset(&state->parts, 0, sizeof(state->parts)); - res = check_part[i++](state, bdev); + res = check_part[i++](state); if (res < 0) { /* We have hit an I/O error which we don't report now. * But record it, and let the others do their job. diff --git a/fs/partitions/check.h b/fs/partitions/check.h index 98dbe1a..4b31a97 100644 --- a/fs/partitions/check.h +++ b/fs/partitions/check.h @@ -6,6 +6,7 @@ * description. */ struct parsed_partitions { + struct block_device *bdev; char name[BDEVNAME_SIZE]; struct { sector_t from; @@ -16,6 +17,12 @@ struct parsed_partitions { int limit; }; +static inline void *read_part_sector(struct parsed_partitions *state, + sector_t n, Sector *p) +{ + return read_dev_sector(state->bdev, n, p); +} + static inline void put_partition(struct parsed_partitions *p, int n, sector_t from, sector_t size) { diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c index 91babda..9e346c1 100644 --- a/fs/partitions/efi.c +++ b/fs/partitions/efi.c @@ -140,8 +140,7 @@ efi_crc32(const void *buf, unsigned long len) * the part[0] entry for this disk, and is the number of * physical sectors available on the disk. */ -static u64 -last_lba(struct block_device *bdev) +static u64 last_lba(struct block_device *bdev) { if (!bdev || !bdev->bd_inode) return 0; @@ -181,27 +180,28 @@ is_pmbr_valid(legacy_mbr *mbr) /** * read_lba(): Read bytes from disk, starting at given LBA - * @bdev + * @state * @lba * @buffer * @size_t * - * Description: Reads @count bytes from @bdev into @buffer. + * Description: Reads @count bytes from @state->bdev into @buffer. * Returns number of bytes read on success, 0 on error. */ -static size_t -read_lba(struct block_device *bdev, u64 lba, u8 * buffer, size_t count) +static size_t read_lba(struct parsed_partitions *state, + u64 lba, u8 *buffer, size_t count) { size_t totalreadcount = 0; + struct block_device *bdev = state->bdev; sector_t n = lba * (bdev_logical_block_size(bdev) / 512); - if (!bdev || !buffer || lba > last_lba(bdev)) + if (!buffer || lba > last_lba(bdev)) return 0; while (count) { int copied = 512; Sector sect; - unsigned char *data = read_dev_sector(bdev, n++, §); + unsigned char *data = read_part_sector(state, n++, §); if (!data) break; if (copied > count) @@ -217,19 +217,20 @@ read_lba(struct block_device *bdev, u64 lba, u8 * buffer, size_t count) /** * alloc_read_gpt_entries(): reads partition entries from disk - * @bdev + * @state * @gpt - GPT header * * Description: Returns ptes on success, NULL on error. * Allocates space for PTEs based on information found in @gpt. * Notes: remember to free pte when you're done! */ -static gpt_entry * -alloc_read_gpt_entries(struct block_device *bdev, gpt_header *gpt) +static gpt_entry *alloc_read_gpt_entries(struct parsed_partitions *state, + gpt_header *gpt) { size_t count; gpt_entry *pte; - if (!bdev || !gpt) + + if (!gpt) return NULL; count = le32_to_cpu(gpt->num_partition_entries) * @@ -240,7 +241,7 @@ alloc_read_gpt_entries(struct block_device *bdev, gpt_header *gpt) if (!pte) return NULL; - if (read_lba(bdev, le64_to_cpu(gpt->partition_entry_lba), + if (read_lba(state, le64_to_cpu(gpt->partition_entry_lba), (u8 *) pte, count) < count) { kfree(pte); @@ -252,27 +253,24 @@ alloc_read_gpt_entries(struct block_device *bdev, gpt_header *gpt) /** * alloc_read_gpt_header(): Allocates GPT header, reads into it from disk - * @bdev + * @state * @lba is the Logical Block Address of the partition table * * Description: returns GPT header on success, NULL on error. Allocates - * and fills a GPT header starting at @ from @bdev. + * and fills a GPT header starting at @ from @state->bdev. * Note: remember to free gpt when finished with it. */ -static gpt_header * -alloc_read_gpt_header(struct block_device *bdev, u64 lba) +static gpt_header *alloc_read_gpt_header(struct parsed_partitions *state, + u64 lba) { gpt_header *gpt; - unsigned ssz = bdev_logical_block_size(bdev); - - if (!bdev) - return NULL; + unsigned ssz = bdev_logical_block_size(state->bdev); gpt = kzalloc(ssz, GFP_KERNEL); if (!gpt) return NULL; - if (read_lba(bdev, lba, (u8 *) gpt, ssz) < ssz) { + if (read_lba(state, lba, (u8 *) gpt, ssz) < ssz) { kfree(gpt); gpt=NULL; return NULL; @@ -283,7 +281,7 @@ alloc_read_gpt_header(struct block_device *bdev, u64 lba) /** * is_gpt_valid() - tests one GPT header and PTEs for validity - * @bdev + * @state * @lba is the logical block address of the GPT header to test * @gpt is a GPT header ptr, filled on return. * @ptes is a PTEs ptr, filled on return. @@ -291,16 +289,15 @@ alloc_read_gpt_header(struct block_device *bdev, u64 lba) * Description: returns 1 if valid, 0 on error. * If valid, returns pointers to newly allocated GPT header and PTEs. */ -static int -is_gpt_valid(struct block_device *bdev, u64 lba, - gpt_header **gpt, gpt_entry **ptes) +static int is_gpt_valid(struct parsed_partitions *state, u64 lba, + gpt_header **gpt, gpt_entry **ptes) { u32 crc, origcrc; u64 lastlba; - if (!bdev || !gpt || !ptes) + if (!ptes) return 0; - if (!(*gpt = alloc_read_gpt_header(bdev, lba))) + if (!(*gpt = alloc_read_gpt_header(state, lba))) return 0; /* Check the GUID Partition Table signature */ @@ -336,7 +333,7 @@ is_gpt_valid(struct block_device *bdev, u64 lba, /* Check the first_usable_lba and last_usable_lba are * within the disk. */ - lastlba = last_lba(bdev); + lastlba = last_lba(state->bdev); if (le64_to_cpu((*gpt)->first_usable_lba) > lastlba) { pr_debug("GPT: first_usable_lba incorrect: %lld > %lld\n", (unsigned long long)le64_to_cpu((*gpt)->first_usable_lba), @@ -350,7 +347,7 @@ is_gpt_valid(struct block_device *bdev, u64 lba, goto fail; } - if (!(*ptes = alloc_read_gpt_entries(bdev, *gpt))) + if (!(*ptes = alloc_read_gpt_entries(state, *gpt))) goto fail; /* Check the GUID Partition Entry Array CRC */ @@ -495,7 +492,7 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba) /** * find_valid_gpt() - Search disk for valid GPT headers and PTEs - * @bdev + * @state * @gpt is a GPT header ptr, filled on return. * @ptes is a PTEs ptr, filled on return. * Description: Returns 1 if valid, 0 on error. @@ -508,24 +505,25 @@ compare_gpts(gpt_header *pgpt, gpt_header *agpt, u64 lastlba) * This protects against devices which misreport their size, and forces * the user to decide to use the Alternate GPT. */ -static int -find_valid_gpt(struct block_device *bdev, gpt_header **gpt, gpt_entry **ptes) +static int find_valid_gpt(struct parsed_partitions *state, gpt_header **gpt, + gpt_entry **ptes) { int good_pgpt = 0, good_agpt = 0, good_pmbr = 0; gpt_header *pgpt = NULL, *agpt = NULL; gpt_entry *pptes = NULL, *aptes = NULL; legacy_mbr *legacymbr; u64 lastlba; - if (!bdev || !gpt || !ptes) + + if (!ptes) return 0; - lastlba = last_lba(bdev); + lastlba = last_lba(state->bdev); if (!force_gpt) { /* This will be added to the EFI Spec. per Intel after v1.02. */ legacymbr = kzalloc(sizeof (*legacymbr), GFP_KERNEL); if (legacymbr) { - read_lba(bdev, 0, (u8 *) legacymbr, - sizeof (*legacymbr)); + read_lba(state, 0, (u8 *) legacymbr, + sizeof (*legacymbr)); good_pmbr = is_pmbr_valid(legacymbr); kfree(legacymbr); } @@ -533,15 +531,14 @@ find_valid_gpt(struct block_device *bdev, gpt_header **gpt, gpt_entry **ptes) goto fail; } - good_pgpt = is_gpt_valid(bdev, GPT_PRIMARY_PARTITION_TABLE_LBA, + good_pgpt = is_gpt_valid(state, GPT_PRIMARY_PARTITION_TABLE_LBA, &pgpt, &pptes); if (good_pgpt) - good_agpt = is_gpt_valid(bdev, + good_agpt = is_gpt_valid(state, le64_to_cpu(pgpt->alternate_lba), &agpt, &aptes); if (!good_agpt && force_gpt) - good_agpt = is_gpt_valid(bdev, lastlba, - &agpt, &aptes); + good_agpt = is_gpt_valid(state, lastlba, &agpt, &aptes); /* The obviously unsuccessful case */ if (!good_pgpt && !good_agpt) @@ -583,9 +580,8 @@ find_valid_gpt(struct block_device *bdev, gpt_header **gpt, gpt_entry **ptes) } /** - * efi_partition(struct parsed_partitions *state, struct block_device *bdev) + * efi_partition(struct parsed_partitions *state) * @state - * @bdev * * Description: called from check.c, if the disk contains GPT * partitions, sets up partition entries in the kernel. @@ -602,15 +598,14 @@ find_valid_gpt(struct block_device *bdev, gpt_header **gpt, gpt_entry **ptes) * 1 if successful * */ -int -efi_partition(struct parsed_partitions *state, struct block_device *bdev) +int efi_partition(struct parsed_partitions *state) { gpt_header *gpt = NULL; gpt_entry *ptes = NULL; u32 i; - unsigned ssz = bdev_logical_block_size(bdev) / 512; + unsigned ssz = bdev_logical_block_size(state->bdev) / 512; - if (!find_valid_gpt(bdev, &gpt, &ptes) || !gpt || !ptes) { + if (!find_valid_gpt(state, &gpt, &ptes) || !gpt || !ptes) { kfree(gpt); kfree(ptes); return 0; @@ -623,7 +618,7 @@ efi_partition(struct parsed_partitions *state, struct block_device *bdev) u64 size = le64_to_cpu(ptes[i].ending_lba) - le64_to_cpu(ptes[i].starting_lba) + 1ULL; - if (!is_pte_valid(&ptes[i], last_lba(bdev))) + if (!is_pte_valid(&ptes[i], last_lba(state->bdev))) continue; put_partition(state, i+1, start * ssz, size * ssz); diff --git a/fs/partitions/efi.h b/fs/partitions/efi.h index 6998b58..b69ab72 100644 --- a/fs/partitions/efi.h +++ b/fs/partitions/efi.h @@ -110,7 +110,7 @@ typedef struct _legacy_mbr { } __attribute__ ((packed)) legacy_mbr; /* Functions */ -extern int efi_partition(struct parsed_partitions *state, struct block_device *bdev); +extern int efi_partition(struct parsed_partitions *state); #endif diff --git a/fs/partitions/ibm.c b/fs/partitions/ibm.c index fc71aab..3e73de5 100644 --- a/fs/partitions/ibm.c +++ b/fs/partitions/ibm.c @@ -58,9 +58,9 @@ cchhb2blk (struct vtoc_cchhb *ptr, struct hd_geometry *geo) { /* */ -int -ibm_partition(struct parsed_partitions *state, struct block_device *bdev) +int ibm_partition(struct parsed_partitions *state) { + struct block_device *bdev = state->bdev; int blocksize, res; loff_t i_size, offset, size, fmt_size; dasd_information2_t *info; @@ -100,7 +100,8 @@ ibm_partition(struct parsed_partitions *state, struct block_device *bdev) /* * Get volume label, extract name and type. */ - data = read_dev_sector(bdev, info->label_block*(blocksize/512), §); + data = read_part_sector(state, info->label_block*(blocksize/512), + §); if (data == NULL) goto out_readerr; @@ -193,8 +194,8 @@ ibm_partition(struct parsed_partitions *state, struct block_device *bdev) */ blk = cchhb2blk(&label->vol.vtoc, geo) + 1; counter = 0; - data = read_dev_sector(bdev, blk * (blocksize/512), - §); + data = read_part_sector(state, blk * (blocksize/512), + §); while (data != NULL) { struct vtoc_format1_label f1; @@ -208,9 +209,8 @@ ibm_partition(struct parsed_partitions *state, struct block_device *bdev) || f1.DS1FMTID == _ascebc['7'] || f1.DS1FMTID == _ascebc['9']) { blk++; - data = read_dev_sector(bdev, blk * - (blocksize/512), - §); + data = read_part_sector(state, + blk * (blocksize/512), §); continue; } @@ -230,9 +230,8 @@ ibm_partition(struct parsed_partitions *state, struct block_device *bdev) size * (blocksize >> 9)); counter++; blk++; - data = read_dev_sector(bdev, - blk * (blocksize/512), - §); + data = read_part_sector(state, + blk * (blocksize/512), §); } if (!data) diff --git a/fs/partitions/ibm.h b/fs/partitions/ibm.h index 31f85a6..08fb080 100644 --- a/fs/partitions/ibm.h +++ b/fs/partitions/ibm.h @@ -1 +1 @@ -int ibm_partition(struct parsed_partitions *, struct block_device *); +int ibm_partition(struct parsed_partitions *); diff --git a/fs/partitions/karma.c b/fs/partitions/karma.c index 176d89b..1cc928b 100644 --- a/fs/partitions/karma.c +++ b/fs/partitions/karma.c @@ -9,7 +9,7 @@ #include "check.h" #include "karma.h" -int karma_partition(struct parsed_partitions *state, struct block_device *bdev) +int karma_partition(struct parsed_partitions *state) { int i; int slot = 1; @@ -29,7 +29,7 @@ int karma_partition(struct parsed_partitions *state, struct block_device *bdev) } __attribute__((packed)) *label; struct d_partition *p; - data = read_dev_sector(bdev, 0, §); + data = read_part_sector(state, 0, §); if (!data) return -1; diff --git a/fs/partitions/karma.h b/fs/partitions/karma.h index ecf7d3f..c764b2e 100644 --- a/fs/partitions/karma.h +++ b/fs/partitions/karma.h @@ -4,5 +4,5 @@ #define KARMA_LABEL_MAGIC 0xAB56 -int karma_partition(struct parsed_partitions *state, struct block_device *bdev); +int karma_partition(struct parsed_partitions *state); diff --git a/fs/partitions/ldm.c b/fs/partitions/ldm.c index 8652fb9..3ceca05 100644 --- a/fs/partitions/ldm.c +++ b/fs/partitions/ldm.c @@ -309,7 +309,7 @@ static bool ldm_compare_tocblocks (const struct tocblock *toc1, /** * ldm_validate_privheads - Compare the primary privhead with its backups - * @bdev: Device holding the LDM Database + * @state: Partition check state including device holding the LDM Database * @ph1: Memory struct to fill with ph contents * * Read and compare all three privheads from disk. @@ -321,8 +321,8 @@ static bool ldm_compare_tocblocks (const struct tocblock *toc1, * Return: 'true' Success * 'false' Error */ -static bool ldm_validate_privheads (struct block_device *bdev, - struct privhead *ph1) +static bool ldm_validate_privheads(struct parsed_partitions *state, + struct privhead *ph1) { static const int off[3] = { OFF_PRIV1, OFF_PRIV2, OFF_PRIV3 }; struct privhead *ph[3] = { ph1 }; @@ -332,7 +332,7 @@ static bool ldm_validate_privheads (struct block_device *bdev, long num_sects; int i; - BUG_ON (!bdev || !ph1); + BUG_ON (!state || !ph1); ph[1] = kmalloc (sizeof (*ph[1]), GFP_KERNEL); ph[2] = kmalloc (sizeof (*ph[2]), GFP_KERNEL); @@ -346,8 +346,8 @@ static bool ldm_validate_privheads (struct block_device *bdev, /* Read and parse privheads */ for (i = 0; i < 3; i++) { - data = read_dev_sector (bdev, - ph[0]->config_start + off[i], §); + data = read_part_sector(state, ph[0]->config_start + off[i], + §); if (!data) { ldm_crit ("Disk read failed."); goto out; @@ -363,7 +363,7 @@ static bool ldm_validate_privheads (struct block_device *bdev, } } - num_sects = bdev->bd_inode->i_size >> 9; + num_sects = state->bdev->bd_inode->i_size >> 9; if ((ph[0]->config_start > num_sects) || ((ph[0]->config_start + ph[0]->config_size) > num_sects)) { @@ -397,20 +397,20 @@ out: /** * ldm_validate_tocblocks - Validate the table of contents and its backups - * @bdev: Device holding the LDM Database - * @base: Offset, into @bdev, of the database + * @state: Partition check state including device holding the LDM Database + * @base: Offset, into @state->bdev, of the database * @ldb: Cache of the database structures * * Find and compare the four tables of contents of the LDM Database stored on - * @bdev and return the parsed information into @toc1. + * @state->bdev and return the parsed information into @toc1. * * The offsets and sizes of the configs are range-checked against a privhead. * * Return: 'true' @toc1 contains validated TOCBLOCK info * 'false' @toc1 contents are undefined */ -static bool ldm_validate_tocblocks(struct block_device *bdev, - unsigned long base, struct ldmdb *ldb) +static bool ldm_validate_tocblocks(struct parsed_partitions *state, + unsigned long base, struct ldmdb *ldb) { static const int off[4] = { OFF_TOCB1, OFF_TOCB2, OFF_TOCB3, OFF_TOCB4}; struct tocblock *tb[4]; @@ -420,7 +420,7 @@ static bool ldm_validate_tocblocks(struct block_device *bdev, int i, nr_tbs; bool result = false; - BUG_ON(!bdev || !ldb); + BUG_ON(!state || !ldb); ph = &ldb->ph; tb[0] = &ldb->toc; tb[1] = kmalloc(sizeof(*tb[1]) * 3, GFP_KERNEL); @@ -437,7 +437,7 @@ static bool ldm_validate_tocblocks(struct block_device *bdev, * skip any that fail as long as we get at least one valid TOCBLOCK. */ for (nr_tbs = i = 0; i < 4; i++) { - data = read_dev_sector(bdev, base + off[i], §); + data = read_part_sector(state, base + off[i], §); if (!data) { ldm_error("Disk read failed for TOCBLOCK %d.", i); continue; @@ -473,7 +473,7 @@ err: /** * ldm_validate_vmdb - Read the VMDB and validate it - * @bdev: Device holding the LDM Database + * @state: Partition check state including device holding the LDM Database * @base: Offset, into @bdev, of the database * @ldb: Cache of the database structures * @@ -483,8 +483,8 @@ err: * Return: 'true' @ldb contains validated VBDB info * 'false' @ldb contents are undefined */ -static bool ldm_validate_vmdb (struct block_device *bdev, unsigned long base, - struct ldmdb *ldb) +static bool ldm_validate_vmdb(struct parsed_partitions *state, + unsigned long base, struct ldmdb *ldb) { Sector sect; u8 *data; @@ -492,12 +492,12 @@ static bool ldm_validate_vmdb (struct block_device *bdev, unsigned long base, struct vmdb *vm; struct tocblock *toc; - BUG_ON (!bdev || !ldb); + BUG_ON (!state || !ldb); vm = &ldb->vm; toc = &ldb->toc; - data = read_dev_sector (bdev, base + OFF_VMDB, §); + data = read_part_sector(state, base + OFF_VMDB, §); if (!data) { ldm_crit ("Disk read failed."); return false; @@ -534,21 +534,21 @@ out: /** * ldm_validate_partition_table - Determine whether bdev might be a dynamic disk - * @bdev: Device holding the LDM Database + * @state: Partition check state including device holding the LDM Database * * This function provides a weak test to decide whether the device is a dynamic * disk or not. It looks for an MS-DOS-style partition table containing at * least one partition of type 0x42 (formerly SFS, now used by Windows for * dynamic disks). * - * N.B. The only possible error can come from the read_dev_sector and that is + * N.B. The only possible error can come from the read_part_sector and that is * only likely to happen if the underlying device is strange. If that IS * the case we should return zero to let someone else try. * - * Return: 'true' @bdev is a dynamic disk - * 'false' @bdev is not a dynamic disk, or an error occurred + * Return: 'true' @state->bdev is a dynamic disk + * 'false' @state->bdev is not a dynamic disk, or an error occurred */ -static bool ldm_validate_partition_table (struct block_device *bdev) +static bool ldm_validate_partition_table(struct parsed_partitions *state) { Sector sect; u8 *data; @@ -556,9 +556,9 @@ static bool ldm_validate_partition_table (struct block_device *bdev) int i; bool result = false; - BUG_ON (!bdev); + BUG_ON(!state); - data = read_dev_sector (bdev, 0, §); + data = read_part_sector(state, 0, §); if (!data) { ldm_crit ("Disk read failed."); return false; @@ -1391,8 +1391,8 @@ static bool ldm_frag_commit (struct list_head *frags, struct ldmdb *ldb) /** * ldm_get_vblks - Read the on-disk database of VBLKs into memory - * @bdev: Device holding the LDM Database - * @base: Offset, into @bdev, of the database + * @state: Partition check state including device holding the LDM Database + * @base: Offset, into @state->bdev, of the database * @ldb: Cache of the database structures * * To use the information from the VBLKs, they need to be read from the disk, @@ -1401,8 +1401,8 @@ static bool ldm_frag_commit (struct list_head *frags, struct ldmdb *ldb) * Return: 'true' All the VBLKs were read successfully * 'false' An error occurred */ -static bool ldm_get_vblks (struct block_device *bdev, unsigned long base, - struct ldmdb *ldb) +static bool ldm_get_vblks(struct parsed_partitions *state, unsigned long base, + struct ldmdb *ldb) { int size, perbuf, skip, finish, s, v, recs; u8 *data = NULL; @@ -1410,7 +1410,7 @@ static bool ldm_get_vblks (struct block_device *bdev, unsigned long base, bool result = false; LIST_HEAD (frags); - BUG_ON (!bdev || !ldb); + BUG_ON(!state || !ldb); size = ldb->vm.vblk_size; perbuf = 512 / size; @@ -1418,7 +1418,7 @@ static bool ldm_get_vblks (struct block_device *bdev, unsigned long base, finish = (size * ldb->vm.last_vblk_seq) >> 9; for (s = skip; s < finish; s++) { /* For each sector */ - data = read_dev_sector (bdev, base + OFF_VMDB + s, §); + data = read_part_sector(state, base + OFF_VMDB + s, §); if (!data) { ldm_crit ("Disk read failed."); goto out; @@ -1474,8 +1474,7 @@ static void ldm_free_vblks (struct list_head *lh) /** * ldm_partition - Find out whether a device is a dynamic disk and handle it - * @pp: List of the partitions parsed so far - * @bdev: Device holding the LDM Database + * @state: Partition check state including device holding the LDM Database * * This determines whether the device @bdev is a dynamic disk and if so creates * the partitions necessary in the gendisk structure pointed to by @hd. @@ -1485,21 +1484,21 @@ static void ldm_free_vblks (struct list_head *lh) * example, if the device is hda, we would have: hda1: LDM database, hda2, hda3, * and so on: the actual data containing partitions. * - * Return: 1 Success, @bdev is a dynamic disk and we handled it - * 0 Success, @bdev is not a dynamic disk + * Return: 1 Success, @state->bdev is a dynamic disk and we handled it + * 0 Success, @state->bdev is not a dynamic disk * -1 An error occurred before enough information had been read - * Or @bdev is a dynamic disk, but it may be corrupted + * Or @state->bdev is a dynamic disk, but it may be corrupted */ -int ldm_partition (struct parsed_partitions *pp, struct block_device *bdev) +int ldm_partition(struct parsed_partitions *state) { struct ldmdb *ldb; unsigned long base; int result = -1; - BUG_ON (!pp || !bdev); + BUG_ON(!state); /* Look for signs of a Dynamic Disk */ - if (!ldm_validate_partition_table (bdev)) + if (!ldm_validate_partition_table(state)) return 0; ldb = kmalloc (sizeof (*ldb), GFP_KERNEL); @@ -1509,15 +1508,15 @@ int ldm_partition (struct parsed_partitions *pp, struct block_device *bdev) } /* Parse and check privheads. */ - if (!ldm_validate_privheads (bdev, &ldb->ph)) + if (!ldm_validate_privheads(state, &ldb->ph)) goto out; /* Already logged */ /* All further references are relative to base (database start). */ base = ldb->ph.config_start; /* Parse and check tocs and vmdb. */ - if (!ldm_validate_tocblocks (bdev, base, ldb) || - !ldm_validate_vmdb (bdev, base, ldb)) + if (!ldm_validate_tocblocks(state, base, ldb) || + !ldm_validate_vmdb(state, base, ldb)) goto out; /* Already logged */ /* Initialize vblk lists in ldmdb struct */ @@ -1527,13 +1526,13 @@ int ldm_partition (struct parsed_partitions *pp, struct block_device *bdev) INIT_LIST_HEAD (&ldb->v_comp); INIT_LIST_HEAD (&ldb->v_part); - if (!ldm_get_vblks (bdev, base, ldb)) { + if (!ldm_get_vblks(state, base, ldb)) { ldm_crit ("Failed to read the VBLKs from the database."); goto cleanup; } /* Finally, create the data partition devices. */ - if (ldm_create_data_partitions (pp, ldb)) { + if (ldm_create_data_partitions(state, ldb)) { ldm_debug ("Parsed LDM database successfully."); result = 1; } diff --git a/fs/partitions/ldm.h b/fs/partitions/ldm.h index 30e08e8..d1fb50b 100644 --- a/fs/partitions/ldm.h +++ b/fs/partitions/ldm.h @@ -209,7 +209,7 @@ struct ldmdb { /* Cache of the database */ struct list_head v_part; }; -int ldm_partition (struct parsed_partitions *state, struct block_device *bdev); +int ldm_partition(struct parsed_partitions *state); #endif /* _FS_PT_LDM_H_ */ diff --git a/fs/partitions/mac.c b/fs/partitions/mac.c index d4a0fad..13e27b0 100644 --- a/fs/partitions/mac.c +++ b/fs/partitions/mac.c @@ -27,7 +27,7 @@ static inline void mac_fix_string(char *stg, int len) stg[i] = 0; } -int mac_partition(struct parsed_partitions *state, struct block_device *bdev) +int mac_partition(struct parsed_partitions *state) { int slot = 1; Sector sect; @@ -42,7 +42,7 @@ int mac_partition(struct parsed_partitions *state, struct block_device *bdev) struct mac_driver_desc *md; /* Get 0th block and look at the first partition map entry. */ - md = (struct mac_driver_desc *) read_dev_sector(bdev, 0, §); + md = read_part_sector(state, 0, §); if (!md) return -1; if (be16_to_cpu(md->signature) != MAC_DRIVER_MAGIC) { @@ -51,7 +51,7 @@ int mac_partition(struct parsed_partitions *state, struct block_device *bdev) } secsize = be16_to_cpu(md->block_size); put_dev_sector(sect); - data = read_dev_sector(bdev, secsize/512, §); + data = read_part_sector(state, secsize/512, §); if (!data) return -1; part = (struct mac_partition *) (data + secsize%512); @@ -64,7 +64,7 @@ int mac_partition(struct parsed_partitions *state, struct block_device *bdev) for (blk = 1; blk <= blocks_in_map; ++blk) { int pos = blk * secsize; put_dev_sector(sect); - data = read_dev_sector(bdev, pos/512, §); + data = read_part_sector(state, pos/512, §); if (!data) return -1; part = (struct mac_partition *) (data + pos%512); @@ -123,7 +123,8 @@ int mac_partition(struct parsed_partitions *state, struct block_device *bdev) } #ifdef CONFIG_PPC_PMAC if (found_root_goodness) - note_bootable_part(bdev->bd_dev, found_root, found_root_goodness); + note_bootable_part(state->bdev->bd_dev, found_root, + found_root_goodness); #endif put_dev_sector(sect); diff --git a/fs/partitions/mac.h b/fs/partitions/mac.h index bbf26e1..3c7d984 100644 --- a/fs/partitions/mac.h +++ b/fs/partitions/mac.h @@ -41,4 +41,4 @@ struct mac_driver_desc { /* ... more stuff */ }; -int mac_partition(struct parsed_partitions *state, struct block_device *bdev); +int mac_partition(struct parsed_partitions *state); diff --git a/fs/partitions/msdos.c b/fs/partitions/msdos.c index 90be97f..645a68d 100644 --- a/fs/partitions/msdos.c +++ b/fs/partitions/msdos.c @@ -64,7 +64,7 @@ msdos_magic_present(unsigned char *p) #define AIX_LABEL_MAGIC2 0xC2 #define AIX_LABEL_MAGIC3 0xD4 #define AIX_LABEL_MAGIC4 0xC1 -static int aix_magic_present(unsigned char *p, struct block_device *bdev) +static int aix_magic_present(struct parsed_partitions *state, unsigned char *p) { struct partition *pt = (struct partition *) (p + 0x1be); Sector sect; @@ -85,7 +85,7 @@ static int aix_magic_present(unsigned char *p, struct block_device *bdev) is_extended_partition(pt)) return 0; } - d = read_dev_sector(bdev, 7, §); + d = read_part_sector(state, 7, §); if (d) { if (d[0] == '_' && d[1] == 'L' && d[2] == 'V' && d[3] == 'M') ret = 1; @@ -105,15 +105,14 @@ static int aix_magic_present(unsigned char *p, struct block_device *bdev) * only for the actual data partitions. */ -static void -parse_extended(struct parsed_partitions *state, struct block_device *bdev, - sector_t first_sector, sector_t first_size) +static void parse_extended(struct parsed_partitions *state, + sector_t first_sector, sector_t first_size) { struct partition *p; Sector sect; unsigned char *data; sector_t this_sector, this_size; - sector_t sector_size = bdev_logical_block_size(bdev) / 512; + sector_t sector_size = bdev_logical_block_size(state->bdev) / 512; int loopct = 0; /* number of links followed without finding a data partition */ int i; @@ -126,7 +125,7 @@ parse_extended(struct parsed_partitions *state, struct block_device *bdev, return; if (state->next == state->limit) return; - data = read_dev_sector(bdev, this_sector, §); + data = read_part_sector(state, this_sector, §); if (!data) return; @@ -198,9 +197,8 @@ done: /* james@bpgc.com: Solaris has a nasty indicator: 0x82 which also indicates linux swap. Be careful before believing this is Solaris. */ -static void -parse_solaris_x86(struct parsed_partitions *state, struct block_device *bdev, - sector_t offset, sector_t size, int origin) +static void parse_solaris_x86(struct parsed_partitions *state, + sector_t offset, sector_t size, int origin) { #ifdef CONFIG_SOLARIS_X86_PARTITION Sector sect; @@ -208,7 +206,7 @@ parse_solaris_x86(struct parsed_partitions *state, struct block_device *bdev, int i; short max_nparts; - v = (struct solaris_x86_vtoc *)read_dev_sector(bdev, offset+1, §); + v = read_part_sector(state, offset + 1, §); if (!v) return; if (le32_to_cpu(v->v_sanity) != SOLARIS_X86_VTOC_SANE) { @@ -245,16 +243,15 @@ parse_solaris_x86(struct parsed_partitions *state, struct block_device *bdev, * Create devices for BSD partitions listed in a disklabel, under a * dos-like partition. See parse_extended() for more information. */ -static void -parse_bsd(struct parsed_partitions *state, struct block_device *bdev, - sector_t offset, sector_t size, int origin, char *flavour, - int max_partitions) +static void parse_bsd(struct parsed_partitions *state, + sector_t offset, sector_t size, int origin, char *flavour, + int max_partitions) { Sector sect; struct bsd_disklabel *l; struct bsd_partition *p; - l = (struct bsd_disklabel *)read_dev_sector(bdev, offset+1, §); + l = read_part_sector(state, offset + 1, §); if (!l) return; if (le32_to_cpu(l->d_magic) != BSD_DISKMAGIC) { @@ -291,33 +288,28 @@ parse_bsd(struct parsed_partitions *state, struct block_device *bdev, } #endif -static void -parse_freebsd(struct parsed_partitions *state, struct block_device *bdev, - sector_t offset, sector_t size, int origin) +static void parse_freebsd(struct parsed_partitions *state, + sector_t offset, sector_t size, int origin) { #ifdef CONFIG_BSD_DISKLABEL - parse_bsd(state, bdev, offset, size, origin, - "bsd", BSD_MAXPARTITIONS); + parse_bsd(state, offset, size, origin, "bsd", BSD_MAXPARTITIONS); #endif } -static void -parse_netbsd(struct parsed_partitions *state, struct block_device *bdev, - sector_t offset, sector_t size, int origin) +static void parse_netbsd(struct parsed_partitions *state, + sector_t offset, sector_t size, int origin) { #ifdef CONFIG_BSD_DISKLABEL - parse_bsd(state, bdev, offset, size, origin, - "netbsd", BSD_MAXPARTITIONS); + parse_bsd(state, offset, size, origin, "netbsd", BSD_MAXPARTITIONS); #endif } -static void -parse_openbsd(struct parsed_partitions *state, struct block_device *bdev, - sector_t offset, sector_t size, int origin) +static void parse_openbsd(struct parsed_partitions *state, + sector_t offset, sector_t size, int origin) { #ifdef CONFIG_BSD_DISKLABEL - parse_bsd(state, bdev, offset, size, origin, - "openbsd", OPENBSD_MAXPARTITIONS); + parse_bsd(state, offset, size, origin, "openbsd", + OPENBSD_MAXPARTITIONS); #endif } @@ -325,16 +317,15 @@ parse_openbsd(struct parsed_partitions *state, struct block_device *bdev, * Create devices for Unixware partitions listed in a disklabel, under a * dos-like partition. See parse_extended() for more information. */ -static void -parse_unixware(struct parsed_partitions *state, struct block_device *bdev, - sector_t offset, sector_t size, int origin) +static void parse_unixware(struct parsed_partitions *state, + sector_t offset, sector_t size, int origin) { #ifdef CONFIG_UNIXWARE_DISKLABEL Sector sect; struct unixware_disklabel *l; struct unixware_slice *p; - l = (struct unixware_disklabel *)read_dev_sector(bdev, offset+29, §); + l = read_part_sector(state, offset + 29, §); if (!l) return; if (le32_to_cpu(l->d_magic) != UNIXWARE_DISKMAGIC || @@ -365,9 +356,8 @@ parse_unixware(struct parsed_partitions *state, struct block_device *bdev, * Anand Krishnamurthy <anandk@wiproge.med.ge.com> * Rajeev V. Pillai <rajeevvp@yahoo.com> */ -static void -parse_minix(struct parsed_partitions *state, struct block_device *bdev, - sector_t offset, sector_t size, int origin) +static void parse_minix(struct parsed_partitions *state, + sector_t offset, sector_t size, int origin) { #ifdef CONFIG_MINIX_SUBPARTITION Sector sect; @@ -375,7 +365,7 @@ parse_minix(struct parsed_partitions *state, struct block_device *bdev, struct partition *p; int i; - data = read_dev_sector(bdev, offset, §); + data = read_part_sector(state, offset, §); if (!data) return; @@ -404,8 +394,7 @@ parse_minix(struct parsed_partitions *state, struct block_device *bdev, static struct { unsigned char id; - void (*parse)(struct parsed_partitions *, struct block_device *, - sector_t, sector_t, int); + void (*parse)(struct parsed_partitions *, sector_t, sector_t, int); } subtypes[] = { {FREEBSD_PARTITION, parse_freebsd}, {NETBSD_PARTITION, parse_netbsd}, @@ -417,16 +406,16 @@ static struct { {0, NULL}, }; -int msdos_partition(struct parsed_partitions *state, struct block_device *bdev) +int msdos_partition(struct parsed_partitions *state) { - sector_t sector_size = bdev_logical_block_size(bdev) / 512; + sector_t sector_size = bdev_logical_block_size(state->bdev) / 512; Sector sect; unsigned char *data; struct partition *p; struct fat_boot_sector *fb; int slot; - data = read_dev_sector(bdev, 0, §); + data = read_part_sector(state, 0, §); if (!data) return -1; if (!msdos_magic_present(data + 510)) { @@ -434,7 +423,7 @@ int msdos_partition(struct parsed_partitions *state, struct block_device *bdev) return 0; } - if (aix_magic_present(data, bdev)) { + if (aix_magic_present(state, data)) { put_dev_sector(sect); printk( " [AIX]"); return 0; @@ -503,7 +492,7 @@ int msdos_partition(struct parsed_partitions *state, struct block_device *bdev) put_partition(state, slot, start, n); printk(" <"); - parse_extended(state, bdev, start, size); + parse_extended(state, start, size); printk(" >"); continue; } @@ -532,8 +521,8 @@ int msdos_partition(struct parsed_partitions *state, struct block_device *bdev) if (!subtypes[n].parse) continue; - subtypes[n].parse(state, bdev, start_sect(p)*sector_size, - nr_sects(p)*sector_size, slot); + subtypes[n].parse(state, start_sect(p) * sector_size, + nr_sects(p) * sector_size, slot); } put_dev_sector(sect); return 1; diff --git a/fs/partitions/msdos.h b/fs/partitions/msdos.h index 01e5e0b..38c781c 100644 --- a/fs/partitions/msdos.h +++ b/fs/partitions/msdos.h @@ -4,5 +4,5 @@ #define MSDOS_LABEL_MAGIC 0xAA55 -int msdos_partition(struct parsed_partitions *state, struct block_device *bdev); +int msdos_partition(struct parsed_partitions *state); diff --git a/fs/partitions/osf.c b/fs/partitions/osf.c index c05c17b..fc22b85 100644 --- a/fs/partitions/osf.c +++ b/fs/partitions/osf.c @@ -10,7 +10,7 @@ #include "check.h" #include "osf.h" -int osf_partition(struct parsed_partitions *state, struct block_device *bdev) +int osf_partition(struct parsed_partitions *state) { int i; int slot = 1; @@ -49,7 +49,7 @@ int osf_partition(struct parsed_partitions *state, struct block_device *bdev) } * label; struct d_partition * partition; - data = read_dev_sector(bdev, 0, §); + data = read_part_sector(state, 0, §); if (!data) return -1; diff --git a/fs/partitions/osf.h b/fs/partitions/osf.h index 427b8ea..20ed231 100644 --- a/fs/partitions/osf.h +++ b/fs/partitions/osf.h @@ -4,4 +4,4 @@ #define DISKLABELMAGIC (0x82564557UL) -int osf_partition(struct parsed_partitions *state, struct block_device *bdev); +int osf_partition(struct parsed_partitions *state); diff --git a/fs/partitions/sgi.c b/fs/partitions/sgi.c index ed5ac83..43b1df9 100644 --- a/fs/partitions/sgi.c +++ b/fs/partitions/sgi.c @@ -27,7 +27,7 @@ struct sgi_disklabel { __be32 _unused1; /* Padding */ }; -int sgi_partition(struct parsed_partitions *state, struct block_device *bdev) +int sgi_partition(struct parsed_partitions *state) { int i, csum; __be32 magic; @@ -39,7 +39,7 @@ int sgi_partition(struct parsed_partitions *state, struct block_device *bdev) struct sgi_partition *p; char b[BDEVNAME_SIZE]; - label = (struct sgi_disklabel *) read_dev_sector(bdev, 0, §); + label = read_part_sector(state, 0, §); if (!label) return -1; p = &label->partitions[0]; @@ -57,7 +57,7 @@ int sgi_partition(struct parsed_partitions *state, struct block_device *bdev) } if(csum) { printk(KERN_WARNING "Dev %s SGI disklabel: csum bad, label corrupted\n", - bdevname(bdev, b)); + bdevname(state->bdev, b)); put_dev_sector(sect); return 0; } diff --git a/fs/partitions/sgi.h b/fs/partitions/sgi.h index 5d5595c..b9553eb 100644 --- a/fs/partitions/sgi.h +++ b/fs/partitions/sgi.h @@ -2,7 +2,7 @@ * fs/partitions/sgi.h */ -extern int sgi_partition(struct parsed_partitions *state, struct block_device *bdev); +extern int sgi_partition(struct parsed_partitions *state); #define SGI_LABEL_MAGIC 0x0be5a941 diff --git a/fs/partitions/sun.c b/fs/partitions/sun.c index c95e6a6..a32660e 100644 --- a/fs/partitions/sun.c +++ b/fs/partitions/sun.c @@ -10,7 +10,7 @@ #include "check.h" #include "sun.h" -int sun_partition(struct parsed_partitions *state, struct block_device *bdev) +int sun_partition(struct parsed_partitions *state) { int i; __be16 csum; @@ -61,7 +61,7 @@ int sun_partition(struct parsed_partitions *state, struct block_device *bdev) int use_vtoc; int nparts; - label = (struct sun_disklabel *)read_dev_sector(bdev, 0, §); + label = read_part_sector(state, 0, §); if (!label) return -1; @@ -78,7 +78,7 @@ int sun_partition(struct parsed_partitions *state, struct block_device *bdev) csum ^= *ush--; if (csum) { printk("Dev %s Sun disklabel: Csum bad, label corrupted\n", - bdevname(bdev, b)); + bdevname(state->bdev, b)); put_dev_sector(sect); return 0; } diff --git a/fs/partitions/sun.h b/fs/partitions/sun.h index 7f864d1..2424baa 100644 --- a/fs/partitions/sun.h +++ b/fs/partitions/sun.h @@ -5,4 +5,4 @@ #define SUN_LABEL_MAGIC 0xDABE #define SUN_VTOC_SANITY 0x600DDEEE -int sun_partition(struct parsed_partitions *state, struct block_device *bdev); +int sun_partition(struct parsed_partitions *state); diff --git a/fs/partitions/sysv68.c b/fs/partitions/sysv68.c index 4eba27b..9030c86 100644 --- a/fs/partitions/sysv68.c +++ b/fs/partitions/sysv68.c @@ -46,7 +46,7 @@ struct slice { }; -int sysv68_partition(struct parsed_partitions *state, struct block_device *bdev) +int sysv68_partition(struct parsed_partitions *state) { int i, slices; int slot = 1; @@ -55,7 +55,7 @@ int sysv68_partition(struct parsed_partitions *state, struct block_device *bdev) struct dkblk0 *b; struct slice *slice; - data = read_dev_sector(bdev, 0, §); + data = read_part_sector(state, 0, §); if (!data) return -1; @@ -68,7 +68,7 @@ int sysv68_partition(struct parsed_partitions *state, struct block_device *bdev) i = be32_to_cpu(b->dk_ios.ios_slcblk); put_dev_sector(sect); - data = read_dev_sector(bdev, i, §); + data = read_part_sector(state, i, §); if (!data) return -1; diff --git a/fs/partitions/sysv68.h b/fs/partitions/sysv68.h index fa733f6..bf2f5ff 100644 --- a/fs/partitions/sysv68.h +++ b/fs/partitions/sysv68.h @@ -1 +1 @@ -extern int sysv68_partition(struct parsed_partitions *state, struct block_device *bdev); +extern int sysv68_partition(struct parsed_partitions *state); diff --git a/fs/partitions/ultrix.c b/fs/partitions/ultrix.c index ec852c1..db9eef2 100644 --- a/fs/partitions/ultrix.c +++ b/fs/partitions/ultrix.c @@ -9,7 +9,7 @@ #include "check.h" #include "ultrix.h" -int ultrix_partition(struct parsed_partitions *state, struct block_device *bdev) +int ultrix_partition(struct parsed_partitions *state) { int i; Sector sect; @@ -26,7 +26,7 @@ int ultrix_partition(struct parsed_partitions *state, struct block_device *bdev) #define PT_MAGIC 0x032957 /* Partition magic number */ #define PT_VALID 1 /* Indicates if struct is valid */ - data = read_dev_sector(bdev, (16384 - sizeof(*label))/512, §); + data = read_part_sector(state, (16384 - sizeof(*label))/512, §); if (!data) return -1; diff --git a/fs/partitions/ultrix.h b/fs/partitions/ultrix.h index a74bf8e..a3cc00b 100644 --- a/fs/partitions/ultrix.h +++ b/fs/partitions/ultrix.h @@ -2,4 +2,4 @@ * fs/partitions/ultrix.h */ -int ultrix_partition(struct parsed_partitions *state, struct block_device *bdev); +int ultrix_partition(struct parsed_partitions *state); -- 1.6.4.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 4/8] block: use struct parsed_partitions *state universally in partition check code 2010-05-15 18:09 ` [PATCH 4/8] block: use struct parsed_partitions *state universally in partition check code Tejun Heo @ 2010-05-16 7:16 ` David Miller 0 siblings, 0 replies; 31+ messages in thread From: David Miller @ 2010-05-16 7:16 UTC (permalink / raw) To: tj Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, bzolnier From: Tejun Heo <tj@kernel.org> Date: Sat, 15 May 2010 20:09:30 +0200 > Make the following changes to partition check code. > > * Add ->bdev to struct parsed_partitions. > > * Introduce read_part_sector() which is a simple wrapper around > read_dev_sector() which takes struct parsed_partitions *state > instead of @bdev. > > * For functions which used to take @state and @bdev, drop @bdev. For > functions which used to take @bdev, replace it with @state. > > * While updating, drop superflous checks on NULL state/bdev in ldm.c. > > This cleans up the API a bit and enables better handling of IO errors > during partition check as the generic partition check code now has > much better visibility into what went wrong in the low level code > paths. > > Signed-off-by: Tejun Heo <tj@kernel.org> Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 5/8] block: improve automatic native capacity unlocking 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo ` (3 preceding siblings ...) 2010-05-15 18:09 ` [PATCH 4/8] block: use struct parsed_partitions *state universally in partition check code Tejun Heo @ 2010-05-15 18:09 ` Tejun Heo 2010-05-16 7:17 ` David Miller 2010-05-15 18:09 ` [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() Tejun Heo ` (4 subsequent siblings) 9 siblings, 1 reply; 31+ messages in thread From: Tejun Heo @ 2010-05-15 18:09 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Cc: Tejun Heo Currently, native capacity unlocking is initiated only when a recognized partition extends beyond the end of the disk. However, there are several other unhandled cases where truncated capacity can lead to misdetection of partitions. * Partition table is fully beyond EOD. * Partition table is partially beyond EOD (daisy chained ones). * Recognized partition starts beyond EOD. This patch updates generic partition check code such that all the above three cases are handled too. For the first two, @state tracks whether low level partition check code tried to read beyond EOD during partition scan and triggers native capacity unlocking accordingly. The third is now handled similarly to the original unlocking case. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Ben Hutchings <ben@decadent.org.uk> --- fs/partitions/check.c | 69 +++++++++++++++++++++++++++++++++++++++---------- fs/partitions/check.h | 5 +++ 2 files changed, 60 insertions(+), 14 deletions(-) diff --git a/fs/partitions/check.c b/fs/partitions/check.c index a19995c..5dcd4b0 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -161,7 +161,7 @@ check_partition(struct gendisk *hd, struct block_device *bdev) struct parsed_partitions *state; int i, res, err; - state = kmalloc(sizeof(struct parsed_partitions), GFP_KERNEL); + state = kzalloc(sizeof(struct parsed_partitions), GFP_KERNEL); if (!state) return NULL; @@ -187,6 +187,8 @@ check_partition(struct gendisk *hd, struct block_device *bdev) } if (res > 0) return state; + if (state->access_beyond_eod) + err = -ENOSPC; if (err) /* The partition is unrecognized. So report I/O errors if there were any */ res = err; @@ -539,13 +541,34 @@ exit: disk_part_iter_exit(&piter); } +static bool disk_unlock_native_capacity(struct gendisk *disk) +{ + const struct block_device_operations *bdops = disk->fops; + + if (bdops->unlock_native_capacity && + !(disk->flags & GENHD_FL_NATIVE_CAPACITY)) { + printk(KERN_CONT "enabling native capacity\n"); + bdops->unlock_native_capacity(disk); + disk->flags |= GENHD_FL_NATIVE_CAPACITY; + return true; + } else { + printk(KERN_CONT "truncated\n"); + return false; + } +} + int rescan_partitions(struct gendisk *disk, struct block_device *bdev) { + struct parsed_partitions *state = NULL; struct disk_part_iter piter; struct hd_struct *part; - struct parsed_partitions *state; int p, highest, res; rescan: + if (state && !IS_ERR(state)) { + kfree(state); + state = NULL; + } + if (bdev->bd_part_count) return -EBUSY; res = invalidate_partition(disk, 0); @@ -563,8 +586,32 @@ rescan: bdev->bd_invalidated = 0; if (!get_capacity(disk) || !(state = check_partition(disk, bdev))) return 0; - if (IS_ERR(state)) /* I/O error reading the partition table */ + if (IS_ERR(state)) { + /* + * I/O error reading the partition table. If any + * partition code tried to read beyond EOD, retry + * after unlocking native capacity. + */ + if (PTR_ERR(state) == -ENOSPC) { + printk(KERN_WARNING "%s: partition table beyond EOD, ", + disk->disk_name); + if (disk_unlock_native_capacity(disk)) + goto rescan; + } return -EIO; + } + /* + * If any partition code tried to read beyond EOD, try + * unlocking native capacity even if partition table is + * sucessfully read as we could be missing some partitions. + */ + if (state->access_beyond_eod) { + printk(KERN_WARNING + "%s: partition table partially beyond EOD, ", + disk->disk_name); + if (disk_unlock_native_capacity(disk)) + goto rescan; + } /* tell userspace that the media / partition table may have changed */ kobject_uevent(&disk_to_dev(disk)->kobj, KOBJ_CHANGE); @@ -590,25 +637,20 @@ rescan: from = state->parts[p].from; if (from >= get_capacity(disk)) { printk(KERN_WARNING - "%s: p%d ignored, start %llu is behind the end of the disk\n", + "%s: p%d start %llu is beyond EOD, ", disk->disk_name, p, (unsigned long long) from); + if (disk_unlock_native_capacity(disk)) + goto rescan; continue; } if (from + size > get_capacity(disk)) { - const struct block_device_operations *bdops = disk->fops; - printk(KERN_WARNING - "%s: p%d size %llu exceeds device capacity, ", + "%s: p%d size %llu extends beyond EOD, ", disk->disk_name, p, (unsigned long long) size); - if (bdops->unlock_native_capacity && - (disk->flags & GENHD_FL_NATIVE_CAPACITY) == 0) { - printk(KERN_CONT "enabling native capacity\n"); - bdops->unlock_native_capacity(disk); - disk->flags |= GENHD_FL_NATIVE_CAPACITY; + if (disk_unlock_native_capacity(disk)) { /* free state and restart */ - kfree(state); goto rescan; } else { /* @@ -617,7 +659,6 @@ rescan: * we limit them to the end of the disk to avoid * creating invalid block devices */ - printk(KERN_CONT "limited to end of disk\n"); size = get_capacity(disk) - from; } } diff --git a/fs/partitions/check.h b/fs/partitions/check.h index 4b31a97..52f8bd3 100644 --- a/fs/partitions/check.h +++ b/fs/partitions/check.h @@ -15,11 +15,16 @@ struct parsed_partitions { } parts[DISK_MAX_PARTS]; int next; int limit; + bool access_beyond_eod; }; static inline void *read_part_sector(struct parsed_partitions *state, sector_t n, Sector *p) { + if (n >= get_capacity(state->bdev->bd_disk)) { + state->access_beyond_eod = true; + return NULL; + } return read_dev_sector(state->bdev, n, p); } -- 1.6.4.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 5/8] block: improve automatic native capacity unlocking 2010-05-15 18:09 ` [PATCH 5/8] block: improve automatic native capacity unlocking Tejun Heo @ 2010-05-16 7:17 ` David Miller 0 siblings, 0 replies; 31+ messages in thread From: David Miller @ 2010-05-16 7:17 UTC (permalink / raw) To: tj Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, bzolnier From: Tejun Heo <tj@kernel.org> Date: Sat, 15 May 2010 20:09:31 +0200 > Currently, native capacity unlocking is initiated only when a > recognized partition extends beyond the end of the disk. However, > there are several other unhandled cases where truncated capacity can > lead to misdetection of partitions. > > * Partition table is fully beyond EOD. > > * Partition table is partially beyond EOD (daisy chained ones). > > * Recognized partition starts beyond EOD. > > This patch updates generic partition check code such that all the > above three cases are handled too. For the first two, @state tracks > whether low level partition check code tried to read beyond EOD during > partition scan and triggers native capacity unlocking accordingly. > The third is now handled similarly to the original unlocking case. > > Signed-off-by: Tejun Heo <tj@kernel.org> Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo ` (4 preceding siblings ...) 2010-05-15 18:09 ` [PATCH 5/8] block: improve automatic native capacity unlocking Tejun Heo @ 2010-05-15 18:09 ` Tejun Heo 2010-05-16 7:18 ` David Miller ` (2 more replies) 2010-05-15 18:09 ` [PATCH 7/8] libata: use the enlarged capacity after late HPA unlock Tejun Heo ` (3 subsequent siblings) 9 siblings, 3 replies; 31+ messages in thread From: Tejun Heo @ 2010-05-15 18:09 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Cc: Tejun Heo Implement sd_unlock_native_capacity() method which calls into hostt->unlock_native_capacity() if implemented. This will be invoked by block layer if partitions extend beyond the end of the device and can be used to implement, for example, on-demand ATA host protected area unlocking. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Ben Hutchings <ben@decadent.org.uk> --- drivers/scsi/sd.c | 22 ++++++++++++++++++++++ include/scsi/scsi_host.h | 8 ++++++++ 2 files changed, 30 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index 8b827f3..b85906e 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -97,6 +97,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC); #endif static int sd_revalidate_disk(struct gendisk *); +static void sd_unlock_native_capacity(struct gendisk *disk); static int sd_probe(struct device *); static int sd_remove(struct device *); static void sd_shutdown(struct device *); @@ -1100,6 +1101,7 @@ static const struct block_device_operations sd_fops = { #endif .media_changed = sd_media_changed, .revalidate_disk = sd_revalidate_disk, + .unlock_native_capacity = sd_unlock_native_capacity, }; static unsigned int sd_completed_bytes(struct scsi_cmnd *scmd) @@ -2101,6 +2103,26 @@ static int sd_revalidate_disk(struct gendisk *disk) } /** + * sd_unlock_native_capacity - unlock native capacity + * @disk: struct gendisk to set capacity for + * + * Block layer calls this function if it detects that partitions + * on @disk reach beyond the end of the device. If the SCSI host + * implements ->unlock_native_capacity() method, it's invoked to + * give it a chance to adjust the device capacity. + * + * CONTEXT: + * Defined by block layer. Might sleep. + */ +static void sd_unlock_native_capacity(struct gendisk *disk) +{ + struct scsi_device *sdev = scsi_disk(disk)->device; + + if (sdev->host->hostt->unlock_native_capacity) + sdev->host->hostt->unlock_native_capacity(sdev); +} + +/** * sd_format_disk_name - format disk name * @prefix: name prefix - ie. "sd" for SCSI disks * @index: index of the disk to format name for diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index c50a97f..b7bdecb 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -327,6 +327,14 @@ struct scsi_host_template { sector_t, int []); /* + * This function is called when one or more partitions on the + * device reach beyond the end of the device. + * + * Status: OPTIONAL + */ + void (*unlock_native_capacity)(struct scsi_device *); + + /* * Can be used to export driver statistics and other infos to the * world outside the kernel ie. userspace and it also provides an * interface to feed the driver with information. -- 1.6.4.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() 2010-05-15 18:09 ` [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() Tejun Heo @ 2010-05-16 7:18 ` David Miller 2010-05-16 13:39 ` James Bottomley 2010-06-02 17:50 ` Jeff Garzik 2 siblings, 0 replies; 31+ messages in thread From: David Miller @ 2010-05-16 7:18 UTC (permalink / raw) To: tj Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, bzolnier From: Tejun Heo <tj@kernel.org> Date: Sat, 15 May 2010 20:09:32 +0200 > Implement sd_unlock_native_capacity() method which calls into > hostt->unlock_native_capacity() if implemented. This will be invoked > by block layer if partitions extend beyond the end of the device and > can be used to implement, for example, on-demand ATA host protected > area unlocking. > > Signed-off-by: Tejun Heo <tj@kernel.org> Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() 2010-05-15 18:09 ` [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() Tejun Heo 2010-05-16 7:18 ` David Miller @ 2010-05-16 13:39 ` James Bottomley 2010-05-16 15:07 ` Ben Hutchings 2010-06-02 17:50 ` Jeff Garzik 2 siblings, 1 reply; 31+ messages in thread From: James Bottomley @ 2010-05-16 13:39 UTC (permalink / raw) To: Tejun Heo Cc: jeff, linux-ide, jens.axboe, linux-scsi, linux-kernel, ben, davem, bzolnier On Sat, 2010-05-15 at 20:09 +0200, Tejun Heo wrote: > Implement sd_unlock_native_capacity() method which calls into > hostt->unlock_native_capacity() if implemented. This will be invoked > by block layer if partitions extend beyond the end of the device and > can be used to implement, for example, on-demand ATA host protected > area unlocking. > > Signed-off-by: Tejun Heo <tj@kernel.org> > Cc: Ben Hutchings <ben@decadent.org.uk> > --- > drivers/scsi/sd.c | 22 ++++++++++++++++++++++ > include/scsi/scsi_host.h | 8 ++++++++ > 2 files changed, 30 insertions(+), 0 deletions(-) > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 8b827f3..b85906e 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -97,6 +97,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC); > #endif > > static int sd_revalidate_disk(struct gendisk *); > +static void sd_unlock_native_capacity(struct gendisk *disk); > static int sd_probe(struct device *); > static int sd_remove(struct device *); > static void sd_shutdown(struct device *); > @@ -1100,6 +1101,7 @@ static const struct block_device_operations sd_fops = { > #endif > .media_changed = sd_media_changed, > .revalidate_disk = sd_revalidate_disk, > + .unlock_native_capacity = sd_unlock_native_capacity, > }; > > static unsigned int sd_completed_bytes(struct scsi_cmnd *scmd) > @@ -2101,6 +2103,26 @@ static int sd_revalidate_disk(struct gendisk *disk) > } > > /** > + * sd_unlock_native_capacity - unlock native capacity > + * @disk: struct gendisk to set capacity for > + * > + * Block layer calls this function if it detects that partitions > + * on @disk reach beyond the end of the device. If the SCSI host > + * implements ->unlock_native_capacity() method, it's invoked to > + * give it a chance to adjust the device capacity. > + * > + * CONTEXT: > + * Defined by block layer. Might sleep. > + */ > +static void sd_unlock_native_capacity(struct gendisk *disk) > +{ > + struct scsi_device *sdev = scsi_disk(disk)->device; > + > + if (sdev->host->hostt->unlock_native_capacity) > + sdev->host->hostt->unlock_native_capacity(sdev); > +} > + > +/** > * sd_format_disk_name - format disk name > * @prefix: name prefix - ie. "sd" for SCSI disks > * @index: index of the disk to format name for > diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h > index c50a97f..b7bdecb 100644 > --- a/include/scsi/scsi_host.h > +++ b/include/scsi/scsi_host.h > @@ -327,6 +327,14 @@ struct scsi_host_template { > sector_t, int []); > > /* > + * This function is called when one or more partitions on the > + * device reach beyond the end of the device. > + * > + * Status: OPTIONAL > + */ > + void (*unlock_native_capacity)(struct scsi_device *); > + > + /* I still principally dislike this threading of the operation up and down the stack. Although we've done it before, we're trying to get away from the paradigm of driver conditions to scsi and scsi conditions to block for all stuff where driver could condition straight to block (alignment is a classic example of the new way of doing things). The reason you need the threading is because the block ops gets hidden at the top of the stack in the upper layers (this was a design choice to try to prevent the lower layers mucking with it). The way we get around this is to use a queue operation instead. As far as I can tell, there's no logical reason for the unlock to be a block op, so lets make it a queue op instead. The consumer in static bool disk_unlock_native_capacity(struct gendisk *disk) then becomes blk_unlock_native_capacity(disk->queue) Block's implementation of this would probably call directly through the queue. You place the usual function in the queue template so the ata slave configure can say blk_queue_unlock_native_function(rq, <unlock_function>) Then there's no need for anything at all in SCSI ... and it becomes a whole lot more obvious how to do legacy ide (if we ever get problems there). James ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() 2010-05-16 13:39 ` James Bottomley @ 2010-05-16 15:07 ` Ben Hutchings 2010-05-16 16:00 ` James Bottomley 0 siblings, 1 reply; 31+ messages in thread From: Ben Hutchings @ 2010-05-16 15:07 UTC (permalink / raw) To: James Bottomley Cc: Tejun Heo, jeff, linux-ide, jens.axboe, linux-scsi, linux-kernel, davem, bzolnier [-- Attachment #1: Type: text/plain, Size: 680 bytes --] On Sun, 2010-05-16 at 08:39 -0500, James Bottomley wrote: [...] > Then there's no need for anything at all in SCSI ... and it becomes a > whole lot more obvious how to do legacy ide (if we ever get problems > there). The block device set_capacity() interface is already defined and was implemented for the IDE disk driver some time ago. The purpose of Tejun's work on sd and libata is to implement this existing interface and so avoid a regression when users switch from IDE to libata-based drivers. I don't see why he should have to replace the interface as well. Ben. -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse. [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() 2010-05-16 15:07 ` Ben Hutchings @ 2010-05-16 16:00 ` James Bottomley 2010-05-16 17:00 ` Tejun Heo 0 siblings, 1 reply; 31+ messages in thread From: James Bottomley @ 2010-05-16 16:00 UTC (permalink / raw) To: Ben Hutchings Cc: Tejun Heo, jeff, linux-ide, jens.axboe, linux-scsi, linux-kernel, davem, bzolnier On Sun, 2010-05-16 at 16:07 +0100, Ben Hutchings wrote: > On Sun, 2010-05-16 at 08:39 -0500, James Bottomley wrote: > [...] > > Then there's no need for anything at all in SCSI ... and it becomes a > > whole lot more obvious how to do legacy ide (if we ever get problems > > there). > > The block device set_capacity() interface is already defined and was > implemented for the IDE disk driver some time ago. The purpose of > Tejun's work on sd and libata is to implement this existing interface > and so avoid a regression when users switch from IDE to libata-based > drivers. I don't see why he should have to replace the interface as > well. I thought we already reached the conclusion that set_capacity wasn't the right interface, hence the redo as a binary lock/unlock of native capacity ... which is revoking the set_capacity interface in this patch. The point I was making is that bdops just works for ide because it's completely monolithic. It gives us a layering problem in SCSI because you have to hook it into the lower layers which don't have access to bdops (since they're an upper layer thing in SCSI). This layering problem is partly the fault of libata ... if we had an ATA native disk driver, it would be able to unlock the capacity on its own. It's just we're using SCSI which has no SAT command it can issue for this, so the functional request has to be pushed down to libata ... leading to the need to thread it through the host template. I was just pointing out that the whole thing is simplified if we use a block queue function approach instead. ide_disk_t has access to the queue, as does gendisk, so it would all "just work" with a simple call site change if we used queue ops instead of block dev ops. The plus side of doing it this way is that the SCSI threading becomes unnecessary: libata gets directly hooked into the unlock function instead of having to do it via an intermediary. James ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() 2010-05-16 16:00 ` James Bottomley @ 2010-05-16 17:00 ` Tejun Heo 2010-05-16 19:23 ` James Bottomley 0 siblings, 1 reply; 31+ messages in thread From: Tejun Heo @ 2010-05-16 17:00 UTC (permalink / raw) To: James Bottomley Cc: Ben Hutchings, jeff, linux-ide, jens.axboe, linux-scsi, linux-kernel, davem, bzolnier Hello, James. On 05/16/2010 06:00 PM, James Bottomley wrote: > This layering problem is partly the fault of libata ... if we had an ATA > native disk driver, it would be able to unlock the capacity on its own. Yeap. > It's just we're using SCSI which has no SAT command it can issue for > this, so the functional request has to be pushed down to libata ... > leading to the need to thread it through the host template. > > I was just pointing out that the whole thing is simplified if we use a > block queue function approach instead. ide_disk_t has access to the > queue, as does gendisk, so it would all "just work" with a simple call > site change if we used queue ops instead of block dev ops. The plus > side of doing it this way is that the SCSI threading becomes > unnecessary: libata gets directly hooked into the unlock function > instead of having to do it via an intermediary. Yeah, it can be made to work via a queue callback but I'm afraid that would be a genuine layering violation (although going through SCSI is extra layering, it isn't really a layering violation). These are request_queue methods. request_fn_proc *request_fn; make_request_fn *make_request_fn; prep_rq_fn *prep_rq_fn; unplug_fn *unplug_fn; merge_bvec_fn *merge_bvec_fn; prepare_flush_fn *prepare_flush_fn; softirq_done_fn *softirq_done_fn; rq_timed_out_fn *rq_timed_out_fn; dma_drain_needed_fn *dma_drain_needed; lld_busy_fn *lld_busy_fn; These are gendisk methods. int (*open) (struct block_device *, fmode_t); int (*release) (struct gendisk *, fmode_t); int (*locked_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); int (*direct_access) (struct block_device *, sector_t, void **, unsigned long *); int (*media_changed) (struct gendisk *); void (*unlock_native_capacity) (struct gendisk *); int (*revalidate_disk) (struct gendisk *); int (*getgeo)(struct block_device *, struct hd_geometry *); request_queue is (or at least supposed to be) oblivious about genhd and its attributes including capacity. After all, request_queue can exist w/o genhd associated, so it would be quite odd to have capacity related method living in request_queue. Another thing is that there is no generic way to reach the associated genhd from request_queue and I can't think of a clean way to map request_queue to the associated ata device w/o in-flight requests (can you even do that from SCSI?). Unfortunately, libata is still properly layered below SCSI, so I'm afraid threading through sd is clumsy yet the cleanest way to do it. Thanks. -- tejun ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() 2010-05-16 17:00 ` Tejun Heo @ 2010-05-16 19:23 ` James Bottomley 2010-05-17 5:30 ` Tejun Heo 0 siblings, 1 reply; 31+ messages in thread From: James Bottomley @ 2010-05-16 19:23 UTC (permalink / raw) To: Tejun Heo Cc: Ben Hutchings, jeff, linux-ide, jens.axboe, linux-scsi, linux-kernel, davem, bzolnier On Sun, 2010-05-16 at 19:00 +0200, Tejun Heo wrote: > Hello, James. > > On 05/16/2010 06:00 PM, James Bottomley wrote: > > This layering problem is partly the fault of libata ... if we had an ATA > > native disk driver, it would be able to unlock the capacity on its own. > > Yeap. > > > It's just we're using SCSI which has no SAT command it can issue for > > this, so the functional request has to be pushed down to libata ... > > leading to the need to thread it through the host template. > > > > I was just pointing out that the whole thing is simplified if we use a > > block queue function approach instead. ide_disk_t has access to the > > queue, as does gendisk, so it would all "just work" with a simple call > > site change if we used queue ops instead of block dev ops. The plus > > side of doing it this way is that the SCSI threading becomes > > unnecessary: libata gets directly hooked into the unlock function > > instead of having to do it via an intermediary. > > Yeah, it can be made to work via a queue callback but I'm afraid that > would be a genuine layering violation (although going through SCSI is > extra layering, it isn't really a layering violation). > > These are request_queue methods. > > request_fn_proc *request_fn; > make_request_fn *make_request_fn; > prep_rq_fn *prep_rq_fn; > unplug_fn *unplug_fn; > merge_bvec_fn *merge_bvec_fn; > prepare_flush_fn *prepare_flush_fn; > softirq_done_fn *softirq_done_fn; > rq_timed_out_fn *rq_timed_out_fn; > dma_drain_needed_fn *dma_drain_needed; > lld_busy_fn *lld_busy_fn; > > These are gendisk methods. > > int (*open) (struct block_device *, fmode_t); > int (*release) (struct gendisk *, fmode_t); > int (*locked_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); > int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); > int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, unsigned long); > int (*direct_access) (struct block_device *, sector_t, > void **, unsigned long *); > int (*media_changed) (struct gendisk *); > void (*unlock_native_capacity) (struct gendisk *); > int (*revalidate_disk) (struct gendisk *); > int (*getgeo)(struct block_device *, struct hd_geometry *); > > request_queue is (or at least supposed to be) oblivious about genhd > and its attributes including capacity. After all, request_queue can > exist w/o genhd associated, so it would be quite odd to have capacity > related method living in request_queue. Yes, I'll sort of buy this ... although it's not quite that clean: barrier methods, which are only used for filesystem above block devices also live in the queue. > Another thing is that there is no generic way to reach the associated > genhd from request_queue and I can't think of a clean way to map > request_queue to the associated ata device w/o in-flight requests (can > you even do that from SCSI?). No ... that's by design ... but you don't need it if all you're doing is unlocking the native capacity (whether on behalf of block dev ops or queue ops). > Unfortunately, libata is still properly layered below SCSI, so I'm > afraid threading through sd is clumsy yet the cleanest way to do it. s/properly/im\&/ but yes, Reluctantly-Acked-by: James Bottomley <James.Bottomley@suse.de> James ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() 2010-05-16 19:23 ` James Bottomley @ 2010-05-17 5:30 ` Tejun Heo 0 siblings, 0 replies; 31+ messages in thread From: Tejun Heo @ 2010-05-17 5:30 UTC (permalink / raw) To: James Bottomley Cc: Ben Hutchings, jeff, linux-ide, jens.axboe, linux-scsi, linux-kernel, davem, bzolnier Hello, On 05/16/2010 09:23 PM, James Bottomley wrote: >> request_queue is (or at least supposed to be) oblivious about genhd >> and its attributes including capacity. After all, request_queue can >> exist w/o genhd associated, so it would be quite odd to have capacity >> related method living in request_queue. > > Yes, I'll sort of buy this ... although it's not quite that clean: > barrier methods, which are only used for filesystem above block devices > also live in the queue. You mean prepare_flush_fn()? Hmmm... >> Another thing is that there is no generic way to reach the associated >> genhd from request_queue and I can't think of a clean way to map >> request_queue to the associated ata device w/o in-flight requests (can >> you even do that from SCSI?). > > No ... that's by design ... but you don't need it if all you're doing is > unlocking the native capacity (whether on behalf of block dev ops or > queue ops). libata defers all those managements stuff to EH and the ata device needs to be accessible to invoke EH. It can be worked around by issuing a pseudo command which is trapped and deferred to EH during command processing but it's much better to be able to access the device directly. >> Unfortunately, libata is still properly layered below SCSI, so I'm >> afraid threading through sd is clumsy yet the cleanest way to do it. > > s/properly/im\&/ Heh, yeah. :-) > but yes, > > Reluctantly-Acked-by: James Bottomley <James.Bottomley@suse.de> Thanks. Much appreciated. -- tejun ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() 2010-05-15 18:09 ` [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() Tejun Heo 2010-05-16 7:18 ` David Miller 2010-05-16 13:39 ` James Bottomley @ 2010-06-02 17:50 ` Jeff Garzik 2 siblings, 0 replies; 31+ messages in thread From: Jeff Garzik @ 2010-06-02 17:50 UTC (permalink / raw) To: Tejun Heo Cc: linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem, bzolnier On 05/15/2010 02:09 PM, Tejun Heo wrote: > Implement sd_unlock_native_capacity() method which calls into > hostt->unlock_native_capacity() if implemented. This will be invoked > by block layer if partitions extend beyond the end of the device and > can be used to implement, for example, on-demand ATA host protected > area unlocking. > > Signed-off-by: Tejun Heo<tj@kernel.org> > Cc: Ben Hutchings<ben@decadent.org.uk> > --- > drivers/scsi/sd.c | 22 ++++++++++++++++++++++ > include/scsi/scsi_host.h | 8 ++++++++ > 2 files changed, 30 insertions(+), 0 deletions(-) > > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c > index 8b827f3..b85906e 100644 > --- a/drivers/scsi/sd.c > +++ b/drivers/scsi/sd.c > @@ -97,6 +97,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_RBC); > #endif > > static int sd_revalidate_disk(struct gendisk *); > +static void sd_unlock_native_capacity(struct gendisk *disk); > static int sd_probe(struct device *); > static int sd_remove(struct device *); > static void sd_shutdown(struct device *); > @@ -1100,6 +1101,7 @@ static const struct block_device_operations sd_fops = { > #endif > .media_changed = sd_media_changed, > .revalidate_disk = sd_revalidate_disk, > + .unlock_native_capacity = sd_unlock_native_capacity, > }; > > static unsigned int sd_completed_bytes(struct scsi_cmnd *scmd) > @@ -2101,6 +2103,26 @@ static int sd_revalidate_disk(struct gendisk *disk) > } > > /** > + * sd_unlock_native_capacity - unlock native capacity > + * @disk: struct gendisk to set capacity for > + * > + * Block layer calls this function if it detects that partitions > + * on @disk reach beyond the end of the device. If the SCSI host > + * implements ->unlock_native_capacity() method, it's invoked to > + * give it a chance to adjust the device capacity. > + * > + * CONTEXT: > + * Defined by block layer. Might sleep. > + */ > +static void sd_unlock_native_capacity(struct gendisk *disk) > +{ > + struct scsi_device *sdev = scsi_disk(disk)->device; > + > + if (sdev->host->hostt->unlock_native_capacity) > + sdev->host->hostt->unlock_native_capacity(sdev); > +} > + > +/** > * sd_format_disk_name - format disk name > * @prefix: name prefix - ie. "sd" for SCSI disks > * @index: index of the disk to format name for > diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h > index c50a97f..b7bdecb 100644 > --- a/include/scsi/scsi_host.h > +++ b/include/scsi/scsi_host.h > @@ -327,6 +327,14 @@ struct scsi_host_template { > sector_t, int []); > > /* > + * This function is called when one or more partitions on the > + * device reach beyond the end of the device. > + * > + * Status: OPTIONAL > + */ > + void (*unlock_native_capacity)(struct scsi_device *); > + > + /* applied 6-8, including this SCSI patch ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 7/8] libata: use the enlarged capacity after late HPA unlock 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo ` (5 preceding siblings ...) 2010-05-15 18:09 ` [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() Tejun Heo @ 2010-05-15 18:09 ` Tejun Heo 2010-05-15 19:22 ` Jeff Garzik 2010-05-16 7:18 ` David Miller 2010-05-15 18:09 ` [PATCH 8/8] libata: implement on-demand HPA unlocking Tejun Heo ` (2 subsequent siblings) 9 siblings, 2 replies; 31+ messages in thread From: Tejun Heo @ 2010-05-15 18:09 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Cc: Tejun Heo After late HPA unlock, libata kept using the original capacity ignoring the new larger native capacity. Enlarging device on the fly doesn't cause any harm. Use the larger native capacity instead. This will enable on-demand HPA unlocking. Signed-off-by: Tejun Heo <tj@kernel.org> Cc: Ben Hutchings <ben@decadent.org.uk> --- drivers/ata/libata-core.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 86f405b..9d6e92d 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -4212,9 +4212,8 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class, 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"); - /* keep using the old n_sectors */ - dev->n_sectors = n_sectors; + "late HPA unlock, n_sectors updated\n"); + /* use the larger n_sectors */ return 0; } -- 1.6.4.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 7/8] libata: use the enlarged capacity after late HPA unlock 2010-05-15 18:09 ` [PATCH 7/8] libata: use the enlarged capacity after late HPA unlock Tejun Heo @ 2010-05-15 19:22 ` Jeff Garzik 2010-05-16 7:18 ` David Miller 1 sibling, 0 replies; 31+ messages in thread From: Jeff Garzik @ 2010-05-15 19:22 UTC (permalink / raw) To: Tejun Heo Cc: linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem, bzolnier On 05/15/2010 02:09 PM, Tejun Heo wrote: > After late HPA unlock, libata kept using the original capacity > ignoring the new larger native capacity. Enlarging device on the fly > doesn't cause any harm. Use the larger native capacity instead. This > will enable on-demand HPA unlocking. > > Signed-off-by: Tejun Heo<tj@kernel.org> > Cc: Ben Hutchings<ben@decadent.org.uk> Acked-by: Jeff Garzik <jgarzik@redhat.com> ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/8] libata: use the enlarged capacity after late HPA unlock 2010-05-15 18:09 ` [PATCH 7/8] libata: use the enlarged capacity after late HPA unlock Tejun Heo 2010-05-15 19:22 ` Jeff Garzik @ 2010-05-16 7:18 ` David Miller 1 sibling, 0 replies; 31+ messages in thread From: David Miller @ 2010-05-16 7:18 UTC (permalink / raw) To: tj Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, bzolnier From: Tejun Heo <tj@kernel.org> Date: Sat, 15 May 2010 20:09:33 +0200 > After late HPA unlock, libata kept using the original capacity > ignoring the new larger native capacity. Enlarging device on the fly > doesn't cause any harm. Use the larger native capacity instead. This > will enable on-demand HPA unlocking. > > Signed-off-by: Tejun Heo <tj@kernel.org> Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 8/8] libata: implement on-demand HPA unlocking 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo ` (6 preceding siblings ...) 2010-05-15 18:09 ` [PATCH 7/8] libata: use the enlarged capacity after late HPA unlock Tejun Heo @ 2010-05-15 18:09 ` Tejun Heo 2010-05-15 19:22 ` Jeff Garzik 2010-05-16 7:19 ` David Miller 2010-05-19 7:05 ` [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo 2010-06-02 16:37 ` Tejun Heo 9 siblings, 2 replies; 31+ messages in thread From: Tejun Heo @ 2010-05-15 18:09 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem Cc: Tejun Heo Implement ata_scsi_unlock_native_capacity() which will be called through SCSI layer when block layer notices that partitions on a device extend beyond the end of the device. It requests EH to unlock HPA, waits for completion and returns the current device capacity. This allows libata to unlock HPA on demand instead of having to decide whether to unlock upfront. Unlocking on demand is safer than unlocking by upfront because some BIOSes write private data to the area beyond HPA limit. This was suggested by Ben Hutchings. Signed-off-by: Tejun Heo <tj@kernel.org> Suggested-by: Ben Hutchings <ben@decadent.org.uk> --- drivers/ata/libata-core.c | 1 + drivers/ata/libata-scsi.c | 29 +++++++++++++++++++++++++++++ include/linux/libata.h | 2 ++ 3 files changed, 32 insertions(+), 0 deletions(-) diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 9d6e92d..b0e379d 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -6797,6 +6797,7 @@ EXPORT_SYMBOL_GPL(ata_dummy_port_info); EXPORT_SYMBOL_GPL(ata_link_next); EXPORT_SYMBOL_GPL(ata_dev_next); EXPORT_SYMBOL_GPL(ata_std_bios_param); +EXPORT_SYMBOL_GPL(ata_scsi_unlock_native_capacity); EXPORT_SYMBOL_GPL(ata_host_init); EXPORT_SYMBOL_GPL(ata_host_alloc); EXPORT_SYMBOL_GPL(ata_host_alloc_pinfo); diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 0088cde..fc51531 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -415,6 +415,35 @@ int ata_std_bios_param(struct scsi_device *sdev, struct block_device *bdev, } /** + * ata_scsi_unlock_native_capacity - unlock native capacity + * @sdev: SCSI device to adjust device capacity for + * + * This function is called if a partition on @sdev extends beyond + * the end of the device. It requests EH to unlock HPA. + * + * LOCKING: + * Defined by the SCSI layer. Might sleep. + */ +void ata_scsi_unlock_native_capacity(struct scsi_device *sdev) +{ + struct ata_port *ap = ata_shost_to_port(sdev->host); + struct ata_device *dev; + unsigned long flags; + + spin_lock_irqsave(ap->lock, flags); + + dev = ata_scsi_find_dev(ap, sdev); + if (dev && dev->n_sectors < dev->n_native_sectors) { + dev->flags |= ATA_DFLAG_UNLOCK_HPA; + dev->link->eh_info.action |= ATA_EH_RESET; + ata_port_schedule_eh(ap); + } + + spin_unlock_irqrestore(ap->lock, flags); + ata_port_wait_eh(ap); +} + +/** * ata_get_identity - Handler for HDIO_GET_IDENTITY ioctl * @ap: target port * @sdev: SCSI device to get identify data for diff --git a/include/linux/libata.h b/include/linux/libata.h index 242eb26..ab7d6de 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -1027,6 +1027,7 @@ extern void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd, extern int ata_std_bios_param(struct scsi_device *sdev, struct block_device *bdev, sector_t capacity, int geom[]); +extern void ata_scsi_unlock_native_capacity(struct scsi_device *sdev); extern int ata_scsi_slave_config(struct scsi_device *sdev); extern void ata_scsi_slave_destroy(struct scsi_device *sdev); extern int ata_scsi_change_queue_depth(struct scsi_device *sdev, @@ -1181,6 +1182,7 @@ extern struct device_attribute *ata_common_sdev_attrs[]; .slave_configure = ata_scsi_slave_config, \ .slave_destroy = ata_scsi_slave_destroy, \ .bios_param = ata_std_bios_param, \ + .unlock_native_capacity = ata_scsi_unlock_native_capacity, \ .sdev_attrs = ata_common_sdev_attrs #define ATA_NCQ_SHT(drv_name) \ -- 1.6.4.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] libata: implement on-demand HPA unlocking 2010-05-15 18:09 ` [PATCH 8/8] libata: implement on-demand HPA unlocking Tejun Heo @ 2010-05-15 19:22 ` Jeff Garzik 2010-05-16 7:19 ` David Miller 1 sibling, 0 replies; 31+ messages in thread From: Jeff Garzik @ 2010-05-15 19:22 UTC (permalink / raw) To: Tejun Heo Cc: linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem, bzolnier On 05/15/2010 02:09 PM, Tejun Heo wrote: > Implement ata_scsi_unlock_native_capacity() which will be called > through SCSI layer when block layer notices that partitions on a > device extend beyond the end of the device. It requests EH to unlock > HPA, waits for completion and returns the current device capacity. > > This allows libata to unlock HPA on demand instead of having to decide > whether to unlock upfront. Unlocking on demand is safer than > unlocking by upfront because some BIOSes write private data to the > area beyond HPA limit. This was suggested by Ben Hutchings. > > Signed-off-by: Tejun Heo<tj@kernel.org> > Suggested-by: Ben Hutchings<ben@decadent.org.uk> Acked-by: Jeff Garzik <jgarzik@redhat.com> ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] libata: implement on-demand HPA unlocking 2010-05-15 18:09 ` [PATCH 8/8] libata: implement on-demand HPA unlocking Tejun Heo 2010-05-15 19:22 ` Jeff Garzik @ 2010-05-16 7:19 ` David Miller 1 sibling, 0 replies; 31+ messages in thread From: David Miller @ 2010-05-16 7:19 UTC (permalink / raw) To: tj Cc: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, bzolnier From: Tejun Heo <tj@kernel.org> Date: Sat, 15 May 2010 20:09:34 +0200 > Implement ata_scsi_unlock_native_capacity() which will be called > through SCSI layer when block layer notices that partitions on a > device extend beyond the end of the device. It requests EH to unlock > HPA, waits for completion and returns the current device capacity. > > This allows libata to unlock HPA on demand instead of having to decide > whether to unlock upfront. Unlocking on demand is safer than > unlocking by upfront because some BIOSes write private data to the > area beyond HPA limit. This was suggested by Ben Hutchings. > > Signed-off-by: Tejun Heo <tj@kernel.org> > Suggested-by: Ben Hutchings <ben@decadent.org.uk> Acked-by: David S. Miller <davem@davemloft.net> ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCHSET] block,libata: implement ->unlock_native_capacity() 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo ` (7 preceding siblings ...) 2010-05-15 18:09 ` [PATCH 8/8] libata: implement on-demand HPA unlocking Tejun Heo @ 2010-05-19 7:05 ` Tejun Heo 2010-05-19 7:08 ` Jens Axboe 2010-06-02 16:37 ` Tejun Heo 9 siblings, 1 reply; 31+ messages in thread From: Tejun Heo @ 2010-05-19 7:05 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem On 05/15/2010 08:09 PM, Tejun Heo wrote: > Hello, all. > > This is the evolved version of the implement-set_capacity patchset[L] > and contains the following patches. > > 0001-buffer-make-invalidate_bdev-drain-all-percpu-LRU-add.patch > 0002-block-restart-partition-scan-after-resizing-a-device.patch > 0003-block-ide-simplify-bdops-set_capacity-to-unlock_nati.patch > 0004-block-use-struct-parsed_partitions-state-universally.patch > 0005-block-improve-automatic-native-capacity-unlocking.patch > 0006-SCSI-implement-sd_unlock_native_capacity.patch > 0007-libata-use-the-enlarged-capacity-after-late-HPA-unlo.patch > 0008-libata-implement-on-demand-HPA-unlocking.patch Other than block part, everything is acked, so how do we route this? 0001-0005 through block tree and 0006-0008 through libata tree seem the easiest unless someone objects. Jens, can you please review 0001-0005 and take them into block tree if it seems okay? Thanks. -- tejun ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCHSET] block,libata: implement ->unlock_native_capacity() 2010-05-19 7:05 ` [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo @ 2010-05-19 7:08 ` Jens Axboe 0 siblings, 0 replies; 31+ messages in thread From: Jens Axboe @ 2010-05-19 7:08 UTC (permalink / raw) To: Tejun Heo Cc: jeff, linux-ide, linux-scsi, James.Bottomley, linux-kernel, ben, davem, bzolnier On Wed, May 19 2010, Tejun Heo wrote: > On 05/15/2010 08:09 PM, Tejun Heo wrote: > > Hello, all. > > > > This is the evolved version of the implement-set_capacity patchset[L] > > and contains the following patches. > > > > 0001-buffer-make-invalidate_bdev-drain-all-percpu-LRU-add.patch > > 0002-block-restart-partition-scan-after-resizing-a-device.patch > > 0003-block-ide-simplify-bdops-set_capacity-to-unlock_nati.patch > > 0004-block-use-struct-parsed_partitions-state-universally.patch > > 0005-block-improve-automatic-native-capacity-unlocking.patch > > 0006-SCSI-implement-sd_unlock_native_capacity.patch > > 0007-libata-use-the-enlarged-capacity-after-late-HPA-unlo.patch > > 0008-libata-implement-on-demand-HPA-unlocking.patch > > Other than block part, everything is acked, so how do we route this? > 0001-0005 through block tree and 0006-0008 through libata tree seem > the easiest unless someone objects. Jens, can you please review > 0001-0005 and take them into block tree if it seems okay? Yeah will do. -- Jens Axboe ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCHSET] block,libata: implement ->unlock_native_capacity() 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo ` (8 preceding siblings ...) 2010-05-19 7:05 ` [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo @ 2010-06-02 16:37 ` Tejun Heo 9 siblings, 0 replies; 31+ messages in thread From: Tejun Heo @ 2010-06-02 16:37 UTC (permalink / raw) To: jeff, linux-ide, jens.axboe, linux-scsi, James.Bottomley, linux-kernel, ben, davem On 05/15/2010 08:09 PM, Tejun Heo wrote: > 0001-buffer-make-invalidate_bdev-drain-all-percpu-LRU-add.patch > 0002-block-restart-partition-scan-after-resizing-a-device.patch > 0003-block-ide-simplify-bdops-set_capacity-to-unlock_nati.patch > 0004-block-use-struct-parsed_partitions-state-universally.patch > 0005-block-improve-automatic-native-capacity-unlocking.patch > 0006-SCSI-implement-sd_unlock_native_capacity.patch > 0007-libata-use-the-enlarged-capacity-after-late-HPA-unlo.patch > 0008-libata-implement-on-demand-HPA-unlocking.patch Jeff, now 0001-0005 are in mainline. Can you please apply 0006-0008? Thanks. -- tejun ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2010-06-02 17:50 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-05-15 18:09 [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo 2010-05-15 18:09 ` [PATCH 1/8] buffer: make invalidate_bdev() drain all percpu LRU add caches Tejun Heo 2010-05-16 7:11 ` David Miller 2010-05-15 18:09 ` [PATCH 2/8] block: restart partition scan after resizing a device Tejun Heo 2010-05-16 7:15 ` David Miller 2010-05-15 18:09 ` [PATCH 3/8] block,ide: simplify bdops->set_capacity() to ->unlock_native_capacity() Tejun Heo 2010-05-15 18:48 ` Bartlomiej Zolnierkiewicz 2010-05-15 18:58 ` Tejun Heo 2010-05-16 7:15 ` David Miller 2010-05-15 18:09 ` [PATCH 4/8] block: use struct parsed_partitions *state universally in partition check code Tejun Heo 2010-05-16 7:16 ` David Miller 2010-05-15 18:09 ` [PATCH 5/8] block: improve automatic native capacity unlocking Tejun Heo 2010-05-16 7:17 ` David Miller 2010-05-15 18:09 ` [PATCH 6/8] SCSI: implement sd_unlock_native_capacity() Tejun Heo 2010-05-16 7:18 ` David Miller 2010-05-16 13:39 ` James Bottomley 2010-05-16 15:07 ` Ben Hutchings 2010-05-16 16:00 ` James Bottomley 2010-05-16 17:00 ` Tejun Heo 2010-05-16 19:23 ` James Bottomley 2010-05-17 5:30 ` Tejun Heo 2010-06-02 17:50 ` Jeff Garzik 2010-05-15 18:09 ` [PATCH 7/8] libata: use the enlarged capacity after late HPA unlock Tejun Heo 2010-05-15 19:22 ` Jeff Garzik 2010-05-16 7:18 ` David Miller 2010-05-15 18:09 ` [PATCH 8/8] libata: implement on-demand HPA unlocking Tejun Heo 2010-05-15 19:22 ` Jeff Garzik 2010-05-16 7:19 ` David Miller 2010-05-19 7:05 ` [PATCHSET] block,libata: implement ->unlock_native_capacity() Tejun Heo 2010-05-19 7:08 ` Jens Axboe 2010-06-02 16:37 ` Tejun Heo
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).