All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PATCH] block,scsi,libata: implement alt_size, take#2
@ 2009-05-09  0:13 ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09  0:13 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen

Hello,

Upon ack, please pull from the following git tree.

  git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-alt_size

This is the second take of implement-alt_size patchset.  Other than
being refreshed to apply on the current block tree, there's no real
change from the last take[L].

Jeff reluctantly acked it and Dan Williams is gonna handle mdadm part
of userland, so I guess it's about time to push this in.

This patchset is on top of the current block#for-2.6.31(f68adec3) and
can be automatically merged with block-peek-fetch branch by pulling.
diffstat follows.

 block/genhd.c              |   26 ++++++++++++++++++++++++++
 drivers/ata/libata-core.c  |    2 ++
 drivers/ata/libata-scsi.c  |    2 ++
 drivers/scsi/sd.c          |    1 +
 include/linux/genhd.h      |   13 ++++++++++++-
 include/linux/libata.h     |    1 +
 include/scsi/scsi_device.h |    1 +
 7 files changed, 45 insertions(+), 1 deletion(-)

Thanks.

--
tejun

[L] http://thread.gmane.org/gmane.linux.kernel.device-mapper.devel/7235

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [GIT PATCH] block,scsi,libata: implement alt_size, take#2
@ 2009-05-09  0:13 ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09  0:13 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen, dm-devel, dan.j.williams

Hello,

Upon ack, please pull from the following git tree.

  git://git.kernel.org/pub/scm/linux/kernel/git/tj/misc.git block-alt_size

This is the second take of implement-alt_size patchset.  Other than
being refreshed to apply on the current block tree, there's no real
change from the last take[L].

Jeff reluctantly acked it and Dan Williams is gonna handle mdadm part
of userland, so I guess it's about time to push this in.

This patchset is on top of the current block#for-2.6.31(f68adec3) and
can be automatically merged with block-peek-fetch branch by pulling.
diffstat follows.

 block/genhd.c              |   26 ++++++++++++++++++++++++++
 drivers/ata/libata-core.c  |    2 ++
 drivers/ata/libata-scsi.c  |    2 ++
 drivers/scsi/sd.c          |    1 +
 include/linux/genhd.h      |   13 ++++++++++++-
 include/linux/libata.h     |    1 +
 include/scsi/scsi_device.h |    1 +
 7 files changed, 45 insertions(+), 1 deletion(-)

Thanks.

--
tejun

[L] http://thread.gmane.org/gmane.linux.kernel.device-mapper.devel/7235

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/3] block: add alt_size
  2009-05-09  0:13 ` Tejun Heo
@ 2009-05-09  0:13   ` Tejun Heo
  -1 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09  0:13 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen
  Cc: Tejun Heo

ATA disks have two sizes - the user visible size and native size.  The
user visible size is usually used by the BIOS to hide certain area
(e.g. recovery partition) or trim size for various reasons.  IDE and
libata can unlock these HPA areas and IDE does so by default and
libata depending on kernel parameters (some distros default to unlock
to maintain compatibility with IDE).

This usually doesn't matter but certain tools need to know what BIOS
wants the disk to look like.  For example, certain BIOS RAID formats
put the metadata at the end of the disk where the 'end' is the trimmed
size, so without knowing the BIOS size, dmraid can't work reliably if
HPA is unlocked.

This patch adds alt_size to genhd and exports it via sysfs.  alt_size
is always smaller than the regular size.  Being a hint, alt_size can
also be set from userland by writing to the sysfs node mostly for
debugging and testing.  Because initialization order isn't enforced,
the alt_size < size limitation is applied while getting the attribute.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Dan Williams <dan.j.williams@gmail.com>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 block/genhd.c         |   26 ++++++++++++++++++++++++++
 include/linux/genhd.h |   13 ++++++++++++-
 2 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 1a4916e..e6a3fdb 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -844,6 +844,29 @@ static ssize_t disk_ro_show(struct device *dev,
 	return sprintf(buf, "%d\n", get_disk_ro(disk) ? 1 : 0);
 }
 
+static ssize_t disk_alt_size_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+
+	return sprintf(buf, "%llu\n",
+		       (unsigned long long)get_alt_capacity(disk));
+}
+
+static ssize_t disk_alt_size_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	sector_t new_size = simple_strtoull(buf, NULL, 0);
+
+	set_alt_capacity(disk, new_size);
+
+	if (get_alt_capacity(disk) == new_size)
+		return count;
+	return -EINVAL;
+}
+
 static ssize_t disk_capability_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -857,6 +880,8 @@ static DEVICE_ATTR(ext_range, S_IRUGO, disk_ext_range_show, NULL);
 static DEVICE_ATTR(removable, S_IRUGO, disk_removable_show, NULL);
 static DEVICE_ATTR(ro, S_IRUGO, disk_ro_show, NULL);
 static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL);
+static DEVICE_ATTR(alt_size, S_IRUGO | S_IWUSR, disk_alt_size_show,
+		   disk_alt_size_store);
 static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
 static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
@@ -875,6 +900,7 @@ static struct attribute *disk_attrs[] = {
 	&dev_attr_removable.attr,
 	&dev_attr_ro.attr,
 	&dev_attr_size.attr,
+	&dev_attr_alt_size.attr,
 	&dev_attr_capability.attr,
 	&dev_attr_stat.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index a1a28ca..a83e4e6 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -159,7 +159,8 @@ struct gendisk {
 
 	struct timer_rand_state *random;
 
-	atomic_t sync_io;		/* RAID */
+	sector_t alt_nr_sects;	/* alt sects, should be <= part0.sects */
+	atomic_t sync_io;	/* RAID */
 	struct work_struct async_notify;
 #ifdef  CONFIG_BLK_DEV_INTEGRITY
 	struct blk_integrity *integrity;
@@ -364,10 +365,20 @@ static inline sector_t get_capacity(struct gendisk *disk)
 {
 	return disk->part0.nr_sects;
 }
+static inline sector_t get_alt_capacity(struct gendisk *disk)
+{
+	if (disk->alt_nr_sects && disk->alt_nr_sects < disk->part0.nr_sects)
+		return disk->alt_nr_sects;
+	return 0;
+}
 static inline void set_capacity(struct gendisk *disk, sector_t size)
 {
 	disk->part0.nr_sects = size;
 }
+static inline void set_alt_capacity(struct gendisk *disk, sector_t size)
+{
+	disk->alt_nr_sects = size;
+}
 
 #ifdef CONFIG_SOLARIS_X86_PARTITION
 
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 1/3] block: add alt_size
@ 2009-05-09  0:13   ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09  0:13 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen, dm-devel, dan.j.williams
  Cc: Tejun Heo

ATA disks have two sizes - the user visible size and native size.  The
user visible size is usually used by the BIOS to hide certain area
(e.g. recovery partition) or trim size for various reasons.  IDE and
libata can unlock these HPA areas and IDE does so by default and
libata depending on kernel parameters (some distros default to unlock
to maintain compatibility with IDE).

This usually doesn't matter but certain tools need to know what BIOS
wants the disk to look like.  For example, certain BIOS RAID formats
put the metadata at the end of the disk where the 'end' is the trimmed
size, so without knowing the BIOS size, dmraid can't work reliably if
HPA is unlocked.

This patch adds alt_size to genhd and exports it via sysfs.  alt_size
is always smaller than the regular size.  Being a hint, alt_size can
also be set from userland by writing to the sysfs node mostly for
debugging and testing.  Because initialization order isn't enforced,
the alt_size < size limitation is applied while getting the attribute.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Dan Williams <dan.j.williams@gmail.com>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 block/genhd.c         |   26 ++++++++++++++++++++++++++
 include/linux/genhd.h |   13 ++++++++++++-
 2 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 1a4916e..e6a3fdb 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -844,6 +844,29 @@ static ssize_t disk_ro_show(struct device *dev,
 	return sprintf(buf, "%d\n", get_disk_ro(disk) ? 1 : 0);
 }
 
+static ssize_t disk_alt_size_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+
+	return sprintf(buf, "%llu\n",
+		       (unsigned long long)get_alt_capacity(disk));
+}
+
+static ssize_t disk_alt_size_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	sector_t new_size = simple_strtoull(buf, NULL, 0);
+
+	set_alt_capacity(disk, new_size);
+
+	if (get_alt_capacity(disk) == new_size)
+		return count;
+	return -EINVAL;
+}
+
 static ssize_t disk_capability_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -857,6 +880,8 @@ static DEVICE_ATTR(ext_range, S_IRUGO, disk_ext_range_show, NULL);
 static DEVICE_ATTR(removable, S_IRUGO, disk_removable_show, NULL);
 static DEVICE_ATTR(ro, S_IRUGO, disk_ro_show, NULL);
 static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL);
+static DEVICE_ATTR(alt_size, S_IRUGO | S_IWUSR, disk_alt_size_show,
+		   disk_alt_size_store);
 static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
 static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
@@ -875,6 +900,7 @@ static struct attribute *disk_attrs[] = {
 	&dev_attr_removable.attr,
 	&dev_attr_ro.attr,
 	&dev_attr_size.attr,
+	&dev_attr_alt_size.attr,
 	&dev_attr_capability.attr,
 	&dev_attr_stat.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index a1a28ca..a83e4e6 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -159,7 +159,8 @@ struct gendisk {
 
 	struct timer_rand_state *random;
 
-	atomic_t sync_io;		/* RAID */
+	sector_t alt_nr_sects;	/* alt sects, should be <= part0.sects */
+	atomic_t sync_io;	/* RAID */
 	struct work_struct async_notify;
 #ifdef  CONFIG_BLK_DEV_INTEGRITY
 	struct blk_integrity *integrity;
@@ -364,10 +365,20 @@ static inline sector_t get_capacity(struct gendisk *disk)
 {
 	return disk->part0.nr_sects;
 }
+static inline sector_t get_alt_capacity(struct gendisk *disk)
+{
+	if (disk->alt_nr_sects && disk->alt_nr_sects < disk->part0.nr_sects)
+		return disk->alt_nr_sects;
+	return 0;
+}
 static inline void set_capacity(struct gendisk *disk, sector_t size)
 {
 	disk->part0.nr_sects = size;
 }
+static inline void set_alt_capacity(struct gendisk *disk, sector_t size)
+{
+	disk->alt_nr_sects = size;
+}
 
 #ifdef CONFIG_SOLARIS_X86_PARTITION
 
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/3] scsi: add scsi_device->alt_capacity
  2009-05-09  0:13 ` Tejun Heo
@ 2009-05-09  0:13   ` Tejun Heo
  -1 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09  0:13 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen
  Cc: Tejun Heo

Add scsi_device->alt_capacity and let sd pass it over to genhd.  This
is to allow SCSI low level drivers to configure alt_capacity via
slave_configure().

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Dan Williams <dan.j.williams@gmail.com>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/sd.c          |    1 +
 include/scsi/scsi_device.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3fcb64b..f3448bf 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1839,6 +1839,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	blk_queue_ordered(sdkp->disk->queue, ordered, sd_prepare_flush);
 
 	set_capacity(disk, sdkp->capacity);
+	set_alt_capacity(disk, sdp->alt_capacity);
 	kfree(buffer);
 
  out:
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 3f566af..b24fdeb 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -92,6 +92,7 @@ struct scsi_device {
 	unsigned int manufacturer;	/* Manufacturer of device, for using 
 					 * vendor-specific cmd's */
 	unsigned sector_size;	/* size in bytes */
+	size_t alt_capacity;	/* alternative capacity, used by sd */
 
 	void *hostdata;		/* available to low-level driver */
 	char type;
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/3] scsi: add scsi_device->alt_capacity
@ 2009-05-09  0:13   ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09  0:13 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen, dm-devel, dan.j.williams
  Cc: Tejun Heo

Add scsi_device->alt_capacity and let sd pass it over to genhd.  This
is to allow SCSI low level drivers to configure alt_capacity via
slave_configure().

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Dan Williams <dan.j.williams@gmail.com>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/sd.c          |    1 +
 include/scsi/scsi_device.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3fcb64b..f3448bf 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1839,6 +1839,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	blk_queue_ordered(sdkp->disk->queue, ordered, sd_prepare_flush);
 
 	set_capacity(disk, sdkp->capacity);
+	set_alt_capacity(disk, sdp->alt_capacity);
 	kfree(buffer);
 
  out:
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 3f566af..b24fdeb 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -92,6 +92,7 @@ struct scsi_device {
 	unsigned int manufacturer;	/* Manufacturer of device, for using 
 					 * vendor-specific cmd's */
 	unsigned sector_size;	/* size in bytes */
+	size_t alt_capacity;	/* alternative capacity, used by sd */
 
 	void *hostdata;		/* available to low-level driver */
 	char type;
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/3] libata: export HPA size as alt_size
  2009-05-09  0:13 ` Tejun Heo
@ 2009-05-09  0:13   ` Tejun Heo
  -1 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09  0:13 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen
  Cc: Tejun Heo

Export HPA size as alt_size so that userland tools which need the BIOS
size can determine it.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Dan Williams <dan.j.williams@gmail.com>
Cc: Jeff Garzik <jeff@garzik.org>
---
 drivers/ata/libata-core.c |    2 ++
 drivers/ata/libata-scsi.c |    2 ++
 include/linux/libata.h    |    1 +
 3 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 17c5d48..4bec0e2 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1536,6 +1536,8 @@ static int ata_hpa_resize(struct ata_device *dev)
 	}
 
 	/* let's unlock HPA */
+	dev->alt_n_sectors = sectors;
+
 	rc = ata_set_max_sectors(dev, native_sectors);
 	if (rc == -EACCES) {
 		/* if device aborted the command, skip HPA resizing */
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2733b0c..6e50cd2 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1127,6 +1127,8 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
 		blk_queue_update_dma_alignment(sdev->request_queue,
 					       ATA_SECT_SIZE - 1);
 		sdev->manage_start_stop = 1;
+
+		sdev->alt_capacity = dev->alt_n_sectors;
 	}
 
 	if (dev->flags & ATA_DFLAG_AN)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3d501db..f7a8327 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -588,6 +588,7 @@ struct ata_device {
 #endif
 	/* n_sector is CLEAR_BEGIN, read comment above CLEAR_BEGIN */
 	u64			n_sectors;	/* size of device, if ATA */
+	u64			alt_n_sectors;	/* size bios wants us to see */
 	unsigned int		class;		/* ATA_DEV_xxx */
 	unsigned long		unpark_deadline;
 
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/3] libata: export HPA size as alt_size
@ 2009-05-09  0:13   ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09  0:13 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen, dm-devel, dan.j.williams
  Cc: Tejun Heo

Export HPA size as alt_size so that userland tools which need the BIOS
size can determine it.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Cc: Dan Williams <dan.j.williams@gmail.com>
Cc: Jeff Garzik <jeff@garzik.org>
---
 drivers/ata/libata-core.c |    2 ++
 drivers/ata/libata-scsi.c |    2 ++
 include/linux/libata.h    |    1 +
 3 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 17c5d48..4bec0e2 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1536,6 +1536,8 @@ static int ata_hpa_resize(struct ata_device *dev)
 	}
 
 	/* let's unlock HPA */
+	dev->alt_n_sectors = sectors;
+
 	rc = ata_set_max_sectors(dev, native_sectors);
 	if (rc == -EACCES) {
 		/* if device aborted the command, skip HPA resizing */
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2733b0c..6e50cd2 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1127,6 +1127,8 @@ static int ata_scsi_dev_config(struct scsi_device *sdev,
 		blk_queue_update_dma_alignment(sdev->request_queue,
 					       ATA_SECT_SIZE - 1);
 		sdev->manage_start_stop = 1;
+
+		sdev->alt_capacity = dev->alt_n_sectors;
 	}
 
 	if (dev->flags & ATA_DFLAG_AN)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3d501db..f7a8327 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -588,6 +588,7 @@ struct ata_device {
 #endif
 	/* n_sector is CLEAR_BEGIN, read comment above CLEAR_BEGIN */
 	u64			n_sectors;	/* size of device, if ATA */
+	u64			alt_n_sectors;	/* size bios wants us to see */
 	unsigned int		class;		/* ATA_DEV_xxx */
 	unsigned long		unpark_deadline;
 
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] scsi: add scsi_device->alt_capacity
  2009-05-09  0:13   ` Tejun Heo
  (?)
@ 2009-05-09  4:23   ` James Bottomley
  2009-05-09 16:09       ` Tejun Heo
  -1 siblings, 1 reply; 24+ messages in thread
From: James Bottomley @ 2009-05-09  4:23 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	Mauelshagen, dm-devel, dan.j.williams

On Sat, 2009-05-09 at 09:13 +0900, Tejun Heo wrote:
> Add scsi_device->alt_capacity and let sd pass it over to genhd.  This
> is to allow SCSI low level drivers to configure alt_capacity via
> slave_configure().
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Cc: Dan Williams <dan.j.williams@gmail.com>
> Cc: Jeff Garzik <jeff@garzik.org>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  drivers/scsi/sd.c          |    1 +
>  include/scsi/scsi_device.h |    1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 3fcb64b..f3448bf 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1839,6 +1839,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>  	blk_queue_ordered(sdkp->disk->queue, ordered, sd_prepare_flush);
>  
>  	set_capacity(disk, sdkp->capacity);
> +	set_alt_capacity(disk, sdp->alt_capacity);
>  	kfree(buffer);
>  
>   out:
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 3f566af..b24fdeb 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -92,6 +92,7 @@ struct scsi_device {
>  	unsigned int manufacturer;	/* Manufacturer of device, for using 
>  					 * vendor-specific cmd's */
>  	unsigned sector_size;	/* size in bytes */
> +	size_t alt_capacity;	/* alternative capacity, used by sd */
>  
>  	void *hostdata;		/* available to low-level driver */
>  	char type;

This is done at slightly the wrong level.  Capacity is actually a
property of struct scsi_disk not struct scsi_device ... shouldn't
alt_capacity be at the same level?

James



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] block: add alt_size
  2009-05-09  0:13   ` Tejun Heo
@ 2009-05-09 13:45     ` Kay Sievers
  -1 siblings, 0 replies; 24+ messages in thread
From: Kay Sievers @ 2009-05-09 13:45 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen, dm-devel, dan.j.williams

On Sat, May 9, 2009 at 02:13, Tejun Heo <tj@kernel.org> wrote:
> This patch adds alt_size to genhd and exports it via sysfs.  alt_size
> is always smaller than the regular size.  Being a hint, alt_size can
> also be set from userland by writing to the sysfs node mostly for
> debugging and testing.

What does "alt_" stand for? I think that should be more descriptive in
an exported interface.

And can we please keep the "size_*" in front of the name, so that they
group together?

Also, values with magic block counts, while there is no way to get the
blocksize with the same interface, are pretty weird. I think the
current "size" attribute is just a bug.
Not sure, how that should be solved, by adding a "blocksize" attribute
that is always in the same context as the current "size*" values, or
by just using bytes for new attributes here.

Almost all tools I've seen using these attributes, have hardcoded *
512 in there, which may cause trouble pretty soon. And this is mostly
a failure of the interface and not of the users, I think.

Thanks,
Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] block: add alt_size
@ 2009-05-09 13:45     ` Kay Sievers
  0 siblings, 0 replies; 24+ messages in thread
From: Kay Sievers @ 2009-05-09 13:45 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen, dm-devel, dan.j.williams

On Sat, May 9, 2009 at 02:13, Tejun Heo <tj@kernel.org> wrote:
> This patch adds alt_size to genhd and exports it via sysfs.  alt_size
> is always smaller than the regular size.  Being a hint, alt_size can
> also be set from userland by writing to the sysfs node mostly for
> debugging and testing.

What does "alt_" stand for? I think that should be more descriptive in
an exported interface.

And can we please keep the "size_*" in front of the name, so that they
group together?

Also, values with magic block counts, while there is no way to get the
blocksize with the same interface, are pretty weird. I think the
current "size" attribute is just a bug.
Not sure, how that should be solved, by adding a "blocksize" attribute
that is always in the same context as the current "size*" values, or
by just using bytes for new attributes here.

Almost all tools I've seen using these attributes, have hardcoded *
512 in there, which may cause trouble pretty soon. And this is mostly
a failure of the interface and not of the users, I think.

Thanks,
Kay

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] block: add alt_size
  2009-05-09 13:45     ` Kay Sievers
@ 2009-05-09 14:04       ` Tejun Heo
  -1 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09 14:04 UTC (permalink / raw)
  To: Kay Sievers
  Cc: jeff, linux-scsi, Mauelshagen, linux-kernel, linux-ide, dm-devel,
	jens.axboe

Hello,

Kay Sievers wrote:
> What does "alt_" stand for? I think that should be more descriptive in
> an exported interface.

Alternative.

> And can we please keep the "size_*" in front of the name, so that they
> group together?

Maybe, but size_alt?  Any better ideas?

> Also, values with magic block counts, while there is no way to get the
> blocksize with the same interface, are pretty weird. I think the
> current "size" attribute is just a bug.

Logical block size is fixed at 512 bytes.  Offset and size are always
represented in multiples of 512 bytes and only get converted to
hardware block size in the lld.

> Not sure, how that should be solved, by adding a "blocksize" attribute
> that is always in the same context as the current "size*" values, or
> by just using bytes for new attributes here.
> 
> Almost all tools I've seen using these attributes, have hardcoded *
> 512 in there, which may cause trouble pretty soon. And this is mostly
> a failure of the interface and not of the users, I think.

No, it will never break.  It will always be 512.  It's there to give
nine bit shift to allow additional 9 bit of addressing without going
to the next full blown bitwidth.  It's chosen to be the lowest common
denominator which gives enough addressing boost to hold things
together till the next bitwidth becomes popular.

For userlevel exporting, it might have been better to use just bytes
there as preformance isn't really an issue, but, well, it's already
determined, so..

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] block: add alt_size
@ 2009-05-09 14:04       ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09 14:04 UTC (permalink / raw)
  To: Kay Sievers
  Cc: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen, dm-devel, dan.j.williams

Hello,

Kay Sievers wrote:
> What does "alt_" stand for? I think that should be more descriptive in
> an exported interface.

Alternative.

> And can we please keep the "size_*" in front of the name, so that they
> group together?

Maybe, but size_alt?  Any better ideas?

> Also, values with magic block counts, while there is no way to get the
> blocksize with the same interface, are pretty weird. I think the
> current "size" attribute is just a bug.

Logical block size is fixed at 512 bytes.  Offset and size are always
represented in multiples of 512 bytes and only get converted to
hardware block size in the lld.

> Not sure, how that should be solved, by adding a "blocksize" attribute
> that is always in the same context as the current "size*" values, or
> by just using bytes for new attributes here.
> 
> Almost all tools I've seen using these attributes, have hardcoded *
> 512 in there, which may cause trouble pretty soon. And this is mostly
> a failure of the interface and not of the users, I think.

No, it will never break.  It will always be 512.  It's there to give
nine bit shift to allow additional 9 bit of addressing without going
to the next full blown bitwidth.  It's chosen to be the lowest common
denominator which gives enough addressing boost to hold things
together till the next bitwidth becomes popular.

For userlevel exporting, it might have been better to use just bytes
there as preformance isn't really an issue, but, well, it's already
determined, so..

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] scsi: add scsi_device->alt_capacity
  2009-05-09  4:23   ` James Bottomley
@ 2009-05-09 16:09       ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09 16:09 UTC (permalink / raw)
  To: James Bottomley
  Cc: jeff, linux-scsi, jens.axboe, linux-kernel, linux-ide, dm-devel,
	Mauelshagen

James Bottomley wrote:
> This is done at slightly the wrong level.  Capacity is actually a
> property of struct scsi_disk not struct scsi_device ... shouldn't
> alt_capacity be at the same level?

Hmmm... I think that was my first try and then I moved it to sdev for
some reason I can't rememer now.  I'll look into it again and try to
move it into sdev.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] scsi: add scsi_device->alt_capacity
@ 2009-05-09 16:09       ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-09 16:09 UTC (permalink / raw)
  To: James Bottomley
  Cc: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	Mauelshagen, dm-devel, dan.j.williams

James Bottomley wrote:
> This is done at slightly the wrong level.  Capacity is actually a
> property of struct scsi_disk not struct scsi_device ... shouldn't
> alt_capacity be at the same level?

Hmmm... I think that was my first try and then I moved it to sdev for
some reason I can't rememer now.  I'll look into it again and try to
move it into sdev.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] scsi: add scsi_device->alt_capacity
  2009-05-09 16:09       ` Tejun Heo
  (?)
@ 2009-05-09 16:23       ` James Bottomley
  2009-05-10  1:26         ` Tejun Heo
  2009-05-15 19:44         ` ATA ULD (was Re: [PATCH 2/3] scsi: add scsi_device->alt_capacity) Jeff Garzik
  -1 siblings, 2 replies; 24+ messages in thread
From: James Bottomley @ 2009-05-09 16:23 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	Mauelshagen, dm-devel, dan.j.williams

On Sun, 2009-05-10 at 01:09 +0900, Tejun Heo wrote:
> James Bottomley wrote:
> > This is done at slightly the wrong level.  Capacity is actually a
> > property of struct scsi_disk not struct scsi_device ... shouldn't
> > alt_capacity be at the same level?
> 
> Hmmm... I think that was my first try and then I moved it to sdev for
> some reason I can't rememer now.  I'll look into it again and try to
> move it into sdev.

Really one of the things I was wondering is why even scsi_disk ...
capacity is in there, but it's also in gendisk, so I've thought
(admittedly never translated it to action) that we could just remove the
duplication in scsi_disk.

This alt_capacity looks to be a pure ATA thing ... I can't find it in
the SCSI specs and there doesn't seem to be a SAT equivalent of the
commands.  Ideally, what should be happening is that the ata ULD would
issue the capacity commands and just set the block alt_capacity without
having to worry about transporting the value up and down the stack.
Matthew Wilcox thought we could begin an implementation of the ATA uld
using the ATA_16 command to transport it through SCSI ... this might
provide the good reason to begin that.

James



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] block: add alt_size
  2009-05-09 14:04       ` Tejun Heo
  (?)
@ 2009-05-09 16:26       ` Kay Sievers
  -1 siblings, 0 replies; 24+ messages in thread
From: Kay Sievers @ 2009-05-09 16:26 UTC (permalink / raw)
  To: Tejun Heo
  Cc: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen, dm-devel, dan.j.williams

On Sat, May 9, 2009 at 16:04, Tejun Heo <tj@kernel.org> wrote:
> Kay Sievers wrote:
>> What does "alt_" stand for? I think that should be more descriptive in
>> an exported interface.
>
> Alternative.
>
>> And can we please keep the "size_*" in front of the name, so that they
>> group together?
>
> Maybe, but size_alt?  Any better ideas?

"size_limit"
"size_restricted"
"size_clipped"
"size_constrain"

anything that ideally would express that this is smaller than the
actual "size", and that is is a "configured" value and not some
hardware property.

>> Also, values with magic block counts, while there is no way to get the
>> blocksize with the same interface, are pretty weird. I think the
>> current "size" attribute is just a bug.
>
> Logical block size is fixed at 512 bytes.  Offset and size are always
> represented in multiples of 512 bytes and only get converted to
> hardware block size in the lld.

Oh, good. Didn't know that this will always be 512, even for devices
with a native size larger than this.

>> Not sure, how that should be solved, by adding a "blocksize" attribute
>> that is always in the same context as the current "size*" values, or
>> by just using bytes for new attributes here.
>>
>> Almost all tools I've seen using these attributes, have hardcoded *
>> 512 in there, which may cause trouble pretty soon. And this is mostly
>> a failure of the interface and not of the users, I think.
>
> No, it will never break.  It will always be 512.

Cool. No problem then. :)

> For userlevel exporting, it might have been better to use just bytes
> there as preformance isn't really an issue, but, well, it's already
> determined, so..

Right, but if it can not change, it's fine, I guess.

Thanks,
Kay

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] scsi: add scsi_device->alt_capacity
  2009-05-09 16:23       ` James Bottomley
@ 2009-05-10  1:26         ` Tejun Heo
  2009-05-15 19:44         ` ATA ULD (was Re: [PATCH 2/3] scsi: add scsi_device->alt_capacity) Jeff Garzik
  1 sibling, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-10  1:26 UTC (permalink / raw)
  To: James Bottomley
  Cc: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	Mauelshagen, dm-devel, dan.j.williams

Hello, James.

James Bottomley wrote:
> On Sun, 2009-05-10 at 01:09 +0900, Tejun Heo wrote:
>> James Bottomley wrote:
>>> This is done at slightly the wrong level.  Capacity is actually a
>>> property of struct scsi_disk not struct scsi_device ... shouldn't
>>> alt_capacity be at the same level?
>> Hmmm... I think that was my first try and then I moved it to sdev for
>> some reason I can't rememer now.  I'll look into it again and try to
>> move it into sdev.
> 
> Really one of the things I was wondering is why even scsi_disk ...
> capacity is in there, but it's also in gendisk, so I've thought
> (admittedly never translated it to action) that we could just remove the
> duplication in scsi_disk.
> 
> This alt_capacity looks to be a pure ATA thing...  I can't find it
> in the SCSI specs and there doesn't seem to be a SAT equivalent of
> the commands.  Ideally, what should be happening is that the ata ULD
> would issue the capacity commands and just set the block
> alt_capacity without having to worry about transporting the value up
> and down the stack.  Matthew Wilcox thought we could begin an
> implementation of the ATA uld using the ATA_16 command to transport
> it through SCSI ... this might provide the good reason to begin
> that.

Yeap, it's a purely ATA specific workaround.

Now I remembered what was the problem.  The problem was that the
latest hook libata can use is ->slave_configure which is called before
HLD attaches, so there's no sdkp or genhd associated during libata
configuration.  Longer term, this definitely is something which should
be stripped out of SCSI whether that happens with ATA HLD on top of
SCSI or a completely separate stack.  Any better ideas on how to work
around this for now?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dm-devel] Re: [PATCH 1/3] block: add alt_size
  2009-05-09 14:04       ` Tejun Heo
  (?)
  (?)
@ 2009-05-11 13:45       ` Konrad Rzeszutek
  2009-05-12  0:53           ` [dm-devel] " Tejun Heo
  -1 siblings, 1 reply; 24+ messages in thread
From: Konrad Rzeszutek @ 2009-05-11 13:45 UTC (permalink / raw)
  To: device-mapper development, tj
  Cc: Kay Sievers, jeff, linux-scsi, Mauelshagen, linux-kernel,
	linux-ide, jens.axboe, martin.petersen

.. snip ..
> > Also, values with magic block counts, while there is no way to get the
> > blocksize with the same interface, are pretty weird. I think the
> > current "size" attribute is just a bug.
> 
> Logical block size is fixed at 512 bytes.  Offset and size are always
> represented in multiples of 512 bytes and only get converted to
> hardware block size in the lld.

That interpretation is at odds with the work that Martin Peterson is
doing with the 4K support. In the e-mail titled: "Re: [PATCH 4 of 8] sd:
Physical block size and alignment support",
Message-ID:<yq1ab67b51p.fsf@sermon.lab.mkp.net> he says:

"
	Konrad> about what a 'logical block', and 'physical block' is
	Konrad> vs. 'hardware sector' ?

	Well, another item on my todo list is to kill the notion of hardware
	sector completely.  The protocols have been referring to logical blocks
	for ages.

	It hasn't been a big problem until now because logical block size has
	been equal to the hardware sector size.  That's no longer a valid
	assumption.
"

Are the ATA/SCSI/etc specs at odds with each other about this?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Re: [PATCH 1/3] block: add alt_size
  2009-05-11 13:45       ` [dm-devel] " Konrad Rzeszutek
@ 2009-05-12  0:53           ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-12  0:53 UTC (permalink / raw)
  To: Konrad Rzeszutek
  Cc: jens.axboe, jeff, linux-scsi, linux-kernel, martin.petersen,
	linux-ide, device-mapper development, Mauelshagen

Konrad Rzeszutek wrote:
> .. snip ..
>>> Also, values with magic block counts, while there is no way to get the
>>> blocksize with the same interface, are pretty weird. I think the
>>> current "size" attribute is just a bug.
>> Logical block size is fixed at 512 bytes.  Offset and size are always
>> represented in multiples of 512 bytes and only get converted to
>> hardware block size in the lld.
> 
> That interpretation is at odds with the work that Martin Peterson is
> doing with the 4K support. In the e-mail titled: "Re: [PATCH 4 of 8] sd:
> Physical block size and alignment support",
> Message-ID:<yq1ab67b51p.fsf@sermon.lab.mkp.net> he says:
> 
> "
> 	Konrad> about what a 'logical block', and 'physical block' is
> 	Konrad> vs. 'hardware sector' ?
> 
> 	Well, another item on my todo list is to kill the notion of hardware
> 	sector completely.  The protocols have been referring to logical blocks
> 	for ages.
> 
> 	It hasn't been a big problem until now because logical block size has
> 	been equal to the hardware sector size.  That's no longer a valid
> 	assumption.
> "
> 
> Are the ATA/SCSI/etc specs at odds with each other about this?

Hardware specs aren't of concern here.  The logical block concept is
there simply to give 9 bit addressing advantage, nothing more, nothing
less.  If hardware's sector size doesn't match it, the lld should be
mapping the sector addresses and sizes and cdrom and a few other
drives have been doing that for ages.  There's nothing new about
devices with sectors larger than 512 bytes.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [dm-devel] Re: [PATCH 1/3] block: add alt_size
@ 2009-05-12  0:53           ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-05-12  0:53 UTC (permalink / raw)
  To: Konrad Rzeszutek
  Cc: device-mapper development, Kay Sievers, jeff, linux-scsi,
	Mauelshagen, linux-kernel, linux-ide, jens.axboe,
	martin.petersen

Konrad Rzeszutek wrote:
> .. snip ..
>>> Also, values with magic block counts, while there is no way to get the
>>> blocksize with the same interface, are pretty weird. I think the
>>> current "size" attribute is just a bug.
>> Logical block size is fixed at 512 bytes.  Offset and size are always
>> represented in multiples of 512 bytes and only get converted to
>> hardware block size in the lld.
> 
> That interpretation is at odds with the work that Martin Peterson is
> doing with the 4K support. In the e-mail titled: "Re: [PATCH 4 of 8] sd:
> Physical block size and alignment support",
> Message-ID:<yq1ab67b51p.fsf@sermon.lab.mkp.net> he says:
> 
> "
> 	Konrad> about what a 'logical block', and 'physical block' is
> 	Konrad> vs. 'hardware sector' ?
> 
> 	Well, another item on my todo list is to kill the notion of hardware
> 	sector completely.  The protocols have been referring to logical blocks
> 	for ages.
> 
> 	It hasn't been a big problem until now because logical block size has
> 	been equal to the hardware sector size.  That's no longer a valid
> 	assumption.
> "
> 
> Are the ATA/SCSI/etc specs at odds with each other about this?

Hardware specs aren't of concern here.  The logical block concept is
there simply to give 9 bit addressing advantage, nothing more, nothing
less.  If hardware's sector size doesn't match it, the lld should be
mapping the sector addresses and sizes and cdrom and a few other
drives have been doing that for ages.  There's nothing new about
devices with sectors larger than 512 bytes.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 24+ messages in thread

* ATA ULD (was Re: [PATCH 2/3] scsi: add scsi_device->alt_capacity)
  2009-05-09 16:23       ` James Bottomley
  2009-05-10  1:26         ` Tejun Heo
@ 2009-05-15 19:44         ` Jeff Garzik
  1 sibling, 0 replies; 24+ messages in thread
From: Jeff Garzik @ 2009-05-15 19:44 UTC (permalink / raw)
  To: James Bottomley
  Cc: Tejun Heo, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	Mauelshagen, dm-devel, dan.j.williams

James Bottomley wrote:
> On Sun, 2009-05-10 at 01:09 +0900, Tejun Heo wrote:
>> James Bottomley wrote:
>>> This is done at slightly the wrong level.  Capacity is actually a
>>> property of struct scsi_disk not struct scsi_device ... shouldn't
>>> alt_capacity be at the same level?
>> Hmmm... I think that was my first try and then I moved it to sdev for
>> some reason I can't rememer now.  I'll look into it again and try to
>> move it into sdev.
> 
> Really one of the things I was wondering is why even scsi_disk ...
> capacity is in there, but it's also in gendisk, so I've thought
> (admittedly never translated it to action) that we could just remove the
> duplication in scsi_disk.
> 
> This alt_capacity looks to be a pure ATA thing ... I can't find it in
> the SCSI specs and there doesn't seem to be a SAT equivalent of the
> commands.  Ideally, what should be happening is that the ata ULD would
> issue the capacity commands and just set the block alt_capacity without
> having to worry about transporting the value up and down the stack.
> Matthew Wilcox thought we could begin an implementation of the ATA uld
> using the ATA_16 command to transport it through SCSI ... this might
> provide the good reason to begin that.

hmmmmm.  Well, ATA ULD is certainly the long term goal.  My main worry 
would then be user impact & confusion, and increasing the difficulty of 
separating libata from SCSI by increasing difficulty of making SCSI 
emulation an optional piece.

At this point the SCSI emulation is not only a hack to get sd.c to do 
work for us, it is also part of the ABI expected of diagnostic tools and 
the like.  That complicates changes.

It is my hope that we can separate libata-scsi.c into a separate kernel 
module, thus at least making it _optional_, thus providing a path for 
CONFIG_ATA + !CONFIG_SCSI.

I do also agree that the ATA hacks can only go on for so long, before 
they start infecting SCSI.   e.g. there was a patch to add DMI check in 
sd.c for an ATA hardware special case.

So far the "ATA infection" has been thankfully limited to things that sd 
might need anyway, like suspend/resume (start/stop)

A "libsd" does not seem viable, though.  Might be better to
	cp sd.c ata-disk.c
and then iterate from there, over time.

	Jeff





^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/3] block: add alt_size
  2009-02-01  2:55 [PATCHSET] block,scsi,libata: implement alt_size Tejun Heo
@ 2009-02-01  2:55   ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-02-01  2:55 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen
  Cc: Tejun Heo

ATA disks have two sizes - the user visible size and native size.  The
user visible size is usually used by the BIOS to hide certain area
(e.g. recovery partition) or trim size for various reasons.  IDE and
libata can unlock these HPA areas and IDE does so by default and
libata depending on kernel parameters (some distros default to unlock
to maintain compatibility with IDE).

This usually doesn't matter but certain tools need to know what BIOS
wants the disk to look like.  For example, certain BIOS RAID formats
put the metadata at the end of the disk where the 'end' is the trimmed
size, so without knowing the BIOS size, dmraid can't work reliably if
HPA is unlocked.

This patch adds alt_size to genhd and exports it via sysfs.  alt_size
is always smaller than the regular size.  Being a hint, alt_size can
also be set from userland by writing to the sysfs node mostly for
debugging and testing.  Because initialization order isn't enforced,
the alt_size < size limitation is applied while getting the attribute.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 block/genhd.c         |   26 ++++++++++++++++++++++++++
 include/linux/genhd.h |   13 ++++++++++++-
 2 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 397960c..db7b501 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -824,6 +824,29 @@ static ssize_t disk_ro_show(struct device *dev,
 	return sprintf(buf, "%d\n", get_disk_ro(disk) ? 1 : 0);
 }
 
+static ssize_t disk_alt_size_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+
+	return sprintf(buf, "%llu\n",
+		       (unsigned long long)get_alt_capacity(disk));
+}
+
+static ssize_t disk_alt_size_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	sector_t new_size = simple_strtoull(buf, NULL, 0);
+
+	set_alt_capacity(disk, new_size);
+
+	if (get_alt_capacity(disk) == new_size)
+		return count;
+	return -EINVAL;
+}
+
 static ssize_t disk_capability_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -837,6 +860,8 @@ static DEVICE_ATTR(ext_range, S_IRUGO, disk_ext_range_show, NULL);
 static DEVICE_ATTR(removable, S_IRUGO, disk_removable_show, NULL);
 static DEVICE_ATTR(ro, S_IRUGO, disk_ro_show, NULL);
 static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL);
+static DEVICE_ATTR(alt_size, S_IRUGO | S_IWUSR, disk_alt_size_show,
+		   disk_alt_size_store);
 static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
 static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
@@ -855,6 +880,7 @@ static struct attribute *disk_attrs[] = {
 	&dev_attr_removable.attr,
 	&dev_attr_ro.attr,
 	&dev_attr_size.attr,
+	&dev_attr_alt_size.attr,
 	&dev_attr_capability.attr,
 	&dev_attr_stat.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 16948ea..763affe 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -159,7 +159,8 @@ struct gendisk {
 
 	struct timer_rand_state *random;
 
-	atomic_t sync_io;		/* RAID */
+	sector_t alt_nr_sects;	/* alt sects, should be <= part0.sects */
+	atomic_t sync_io;	/* RAID */
 	struct work_struct async_notify;
 #ifdef  CONFIG_BLK_DEV_INTEGRITY
 	struct blk_integrity *integrity;
@@ -364,10 +365,20 @@ static inline sector_t get_capacity(struct gendisk *disk)
 {
 	return disk->part0.nr_sects;
 }
+static inline sector_t get_alt_capacity(struct gendisk *disk)
+{
+	if (disk->alt_nr_sects && disk->alt_nr_sects < disk->part0.nr_sects)
+		return disk->alt_nr_sects;
+	return 0;
+}
 static inline void set_capacity(struct gendisk *disk, sector_t size)
 {
 	disk->part0.nr_sects = size;
 }
+static inline void set_alt_capacity(struct gendisk *disk, sector_t size)
+{
+	disk->alt_nr_sects = size;
+}
 
 #ifdef CONFIG_SOLARIS_X86_PARTITION
 
-- 
1.6.0.2

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 1/3] block: add alt_size
@ 2009-02-01  2:55   ` Tejun Heo
  0 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2009-02-01  2:55 UTC (permalink / raw)
  To: jeff, linux-ide, jens.axboe, linux-kernel, linux-scsi,
	James.Bottomley, Mauelshagen, dm-devel
  Cc: Tejun Heo

ATA disks have two sizes - the user visible size and native size.  The
user visible size is usually used by the BIOS to hide certain area
(e.g. recovery partition) or trim size for various reasons.  IDE and
libata can unlock these HPA areas and IDE does so by default and
libata depending on kernel parameters (some distros default to unlock
to maintain compatibility with IDE).

This usually doesn't matter but certain tools need to know what BIOS
wants the disk to look like.  For example, certain BIOS RAID formats
put the metadata at the end of the disk where the 'end' is the trimmed
size, so without knowing the BIOS size, dmraid can't work reliably if
HPA is unlocked.

This patch adds alt_size to genhd and exports it via sysfs.  alt_size
is always smaller than the regular size.  Being a hint, alt_size can
also be set from userland by writing to the sysfs node mostly for
debugging and testing.  Because initialization order isn't enforced,
the alt_size < size limitation is applied while getting the attribute.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 block/genhd.c         |   26 ++++++++++++++++++++++++++
 include/linux/genhd.h |   13 ++++++++++++-
 2 files changed, 38 insertions(+), 1 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 397960c..db7b501 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -824,6 +824,29 @@ static ssize_t disk_ro_show(struct device *dev,
 	return sprintf(buf, "%d\n", get_disk_ro(disk) ? 1 : 0);
 }
 
+static ssize_t disk_alt_size_show(struct device *dev,
+				  struct device_attribute *attr, char *buf)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+
+	return sprintf(buf, "%llu\n",
+		       (unsigned long long)get_alt_capacity(disk));
+}
+
+static ssize_t disk_alt_size_store(struct device *dev,
+				   struct device_attribute *attr,
+				   const char *buf, size_t count)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+	sector_t new_size = simple_strtoull(buf, NULL, 0);
+
+	set_alt_capacity(disk, new_size);
+
+	if (get_alt_capacity(disk) == new_size)
+		return count;
+	return -EINVAL;
+}
+
 static ssize_t disk_capability_show(struct device *dev,
 				    struct device_attribute *attr, char *buf)
 {
@@ -837,6 +860,8 @@ static DEVICE_ATTR(ext_range, S_IRUGO, disk_ext_range_show, NULL);
 static DEVICE_ATTR(removable, S_IRUGO, disk_removable_show, NULL);
 static DEVICE_ATTR(ro, S_IRUGO, disk_ro_show, NULL);
 static DEVICE_ATTR(size, S_IRUGO, part_size_show, NULL);
+static DEVICE_ATTR(alt_size, S_IRUGO | S_IWUSR, disk_alt_size_show,
+		   disk_alt_size_store);
 static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
 static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
@@ -855,6 +880,7 @@ static struct attribute *disk_attrs[] = {
 	&dev_attr_removable.attr,
 	&dev_attr_ro.attr,
 	&dev_attr_size.attr,
+	&dev_attr_alt_size.attr,
 	&dev_attr_capability.attr,
 	&dev_attr_stat.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 16948ea..763affe 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -159,7 +159,8 @@ struct gendisk {
 
 	struct timer_rand_state *random;
 
-	atomic_t sync_io;		/* RAID */
+	sector_t alt_nr_sects;	/* alt sects, should be <= part0.sects */
+	atomic_t sync_io;	/* RAID */
 	struct work_struct async_notify;
 #ifdef  CONFIG_BLK_DEV_INTEGRITY
 	struct blk_integrity *integrity;
@@ -364,10 +365,20 @@ static inline sector_t get_capacity(struct gendisk *disk)
 {
 	return disk->part0.nr_sects;
 }
+static inline sector_t get_alt_capacity(struct gendisk *disk)
+{
+	if (disk->alt_nr_sects && disk->alt_nr_sects < disk->part0.nr_sects)
+		return disk->alt_nr_sects;
+	return 0;
+}
 static inline void set_capacity(struct gendisk *disk, sector_t size)
 {
 	disk->part0.nr_sects = size;
 }
+static inline void set_alt_capacity(struct gendisk *disk, sector_t size)
+{
+	disk->alt_nr_sects = size;
+}
 
 #ifdef CONFIG_SOLARIS_X86_PARTITION
 
-- 
1.6.0.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2009-05-15 19:44 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-05-09  0:13 [GIT PATCH] block,scsi,libata: implement alt_size, take#2 Tejun Heo
2009-05-09  0:13 ` Tejun Heo
2009-05-09  0:13 ` [PATCH 1/3] block: add alt_size Tejun Heo
2009-05-09  0:13   ` Tejun Heo
2009-05-09 13:45   ` Kay Sievers
2009-05-09 13:45     ` Kay Sievers
2009-05-09 14:04     ` Tejun Heo
2009-05-09 14:04       ` Tejun Heo
2009-05-09 16:26       ` Kay Sievers
2009-05-11 13:45       ` [dm-devel] " Konrad Rzeszutek
2009-05-12  0:53         ` Tejun Heo
2009-05-12  0:53           ` [dm-devel] " Tejun Heo
2009-05-09  0:13 ` [PATCH 2/3] scsi: add scsi_device->alt_capacity Tejun Heo
2009-05-09  0:13   ` Tejun Heo
2009-05-09  4:23   ` James Bottomley
2009-05-09 16:09     ` Tejun Heo
2009-05-09 16:09       ` Tejun Heo
2009-05-09 16:23       ` James Bottomley
2009-05-10  1:26         ` Tejun Heo
2009-05-15 19:44         ` ATA ULD (was Re: [PATCH 2/3] scsi: add scsi_device->alt_capacity) Jeff Garzik
2009-05-09  0:13 ` [PATCH 3/3] libata: export HPA size as alt_size Tejun Heo
2009-05-09  0:13   ` Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2009-02-01  2:55 [PATCHSET] block,scsi,libata: implement alt_size Tejun Heo
2009-02-01  2:55 ` [PATCH 1/3] block: add alt_size Tejun Heo
2009-02-01  2:55   ` Tejun Heo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.