RE: [PATCH 2/4] pmem: Add support for getgeo()

2014-11-03 Thread Wilcox, Matthew R
I agree that there should be a generic fake getgeo routine; but fixing that is 
a topic for a different patchset and it doesn't need to get folded into this 
driver submission process.

-Original Message-
From: Elliott, Robert (Server Storage) [mailto:elli...@hp.com] 
Sent: Saturday, November 01, 2014 8:28 PM
To: Ross Zwisler; Jens Axboe; Wilcox, Matthew R; Boaz Harrosh; Nick Piggin; 
Kani, Toshimitsu; Knippers, Linda; linux-fsde...@vger.kernel.org; 
linux-kernel@vger.kernel.org; linux-nvd...@lists.01.org
Subject: RE: [PATCH 2/4] pmem: Add support for getgeo()



> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
> ow...@vger.kernel.org] On Behalf Of Ross Zwisler
> Sent: Wednesday, 27 August, 2014 4:12 PM
> To: Jens Axboe; Matthew Wilcox; Boaz Harrosh; Nick Piggin; linux-
> fsde...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> nvd...@lists.01.org
> Cc: Ross Zwisler
> Subject: [PATCH 2/4] pmem: Add support for getgeo()
> 
> Some programs require HDIO_GETGEO work, which requires we implement
> getgeo.  Based off of the work done to the NVMe driver in this
> commit:
> 
> commit 4cc09e2dc4cb ("NVMe: Add getgeo to block ops")
> 
> Signed-off-by: Ross Zwisler 
> ---
>  drivers/block/pmem.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/block/pmem.c b/drivers/block/pmem.c
> index d366b9b..60bbe0d 100644
> --- a/drivers/block/pmem.c
> +++ b/drivers/block/pmem.c
> @@ -50,6 +50,15 @@ struct pmem_device {
>   size_t  size;
>  };
> 
> +static int pmem_getgeo(struct block_device *bd, struct hd_geometry
> *geo)
> +{
> + /* some standard values */
> + geo->heads = 1 << 6;
> + geo->sectors = 1 << 5;
> + geo->cylinders = get_capacity(bd->bd_disk) >> 11;

Just stuffing the result of get_capacity into the 16-bit 
cylinders field will overflow/wrap on large capacities.
0x << 11 = 0x7FF_F800 = 64 GiB (68.7 GB)

How many programs still need these meaningless fields?
Could the bogus information be created elsewhere so
each block driver doesn't need to do this?


> + return 0;
> +}
> +
>  /*
>   * direct translation from (pmem,sector) => void*
>   * We do not require that sector be page aligned.
> @@ -176,6 +185,7 @@ out:
> 
>  static const struct block_device_operations pmem_fops = {
>   .owner =THIS_MODULE,
> + .getgeo =   pmem_getgeo,
>  };
> 
>  /* Kernel module stuff */
> --


---
Rob ElliottHP Server Storage




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 2/4] pmem: Add support for getgeo()

2014-11-03 Thread Wilcox, Matthew R
I agree that there should be a generic fake getgeo routine; but fixing that is 
a topic for a different patchset and it doesn't need to get folded into this 
driver submission process.

-Original Message-
From: Elliott, Robert (Server Storage) [mailto:elli...@hp.com] 
Sent: Saturday, November 01, 2014 8:28 PM
To: Ross Zwisler; Jens Axboe; Wilcox, Matthew R; Boaz Harrosh; Nick Piggin; 
Kani, Toshimitsu; Knippers, Linda; linux-fsde...@vger.kernel.org; 
linux-kernel@vger.kernel.org; linux-nvd...@lists.01.org
Subject: RE: [PATCH 2/4] pmem: Add support for getgeo()



 -Original Message-
 From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
 ow...@vger.kernel.org] On Behalf Of Ross Zwisler
 Sent: Wednesday, 27 August, 2014 4:12 PM
 To: Jens Axboe; Matthew Wilcox; Boaz Harrosh; Nick Piggin; linux-
 fsde...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
 nvd...@lists.01.org
 Cc: Ross Zwisler
 Subject: [PATCH 2/4] pmem: Add support for getgeo()
 
 Some programs require HDIO_GETGEO work, which requires we implement
 getgeo.  Based off of the work done to the NVMe driver in this
 commit:
 
 commit 4cc09e2dc4cb (NVMe: Add getgeo to block ops)
 
 Signed-off-by: Ross Zwisler ross.zwis...@linux.intel.com
 ---
  drivers/block/pmem.c | 10 ++
  1 file changed, 10 insertions(+)
 
 diff --git a/drivers/block/pmem.c b/drivers/block/pmem.c
 index d366b9b..60bbe0d 100644
 --- a/drivers/block/pmem.c
 +++ b/drivers/block/pmem.c
 @@ -50,6 +50,15 @@ struct pmem_device {
   size_t  size;
  };
 
 +static int pmem_getgeo(struct block_device *bd, struct hd_geometry
 *geo)
 +{
 + /* some standard values */
 + geo-heads = 1  6;
 + geo-sectors = 1  5;
 + geo-cylinders = get_capacity(bd-bd_disk)  11;

Just stuffing the result of get_capacity into the 16-bit 
cylinders field will overflow/wrap on large capacities.
0x  11 = 0x7FF_F800 = 64 GiB (68.7 GB)

How many programs still need these meaningless fields?
Could the bogus information be created elsewhere so
each block driver doesn't need to do this?


 + return 0;
 +}
 +
  /*
   * direct translation from (pmem,sector) = void*
   * We do not require that sector be page aligned.
 @@ -176,6 +185,7 @@ out:
 
  static const struct block_device_operations pmem_fops = {
   .owner =THIS_MODULE,
 + .getgeo =   pmem_getgeo,
  };
 
  /* Kernel module stuff */
 --


---
Rob ElliottHP Server Storage




--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 2/4] pmem: Add support for getgeo()

2014-11-01 Thread Elliott, Robert (Server Storage)


> -Original Message-
> From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
> ow...@vger.kernel.org] On Behalf Of Ross Zwisler
> Sent: Wednesday, 27 August, 2014 4:12 PM
> To: Jens Axboe; Matthew Wilcox; Boaz Harrosh; Nick Piggin; linux-
> fsde...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> nvd...@lists.01.org
> Cc: Ross Zwisler
> Subject: [PATCH 2/4] pmem: Add support for getgeo()
> 
> Some programs require HDIO_GETGEO work, which requires we implement
> getgeo.  Based off of the work done to the NVMe driver in this
> commit:
> 
> commit 4cc09e2dc4cb ("NVMe: Add getgeo to block ops")
> 
> Signed-off-by: Ross Zwisler 
> ---
>  drivers/block/pmem.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/block/pmem.c b/drivers/block/pmem.c
> index d366b9b..60bbe0d 100644
> --- a/drivers/block/pmem.c
> +++ b/drivers/block/pmem.c
> @@ -50,6 +50,15 @@ struct pmem_device {
>   size_t  size;
>  };
> 
> +static int pmem_getgeo(struct block_device *bd, struct hd_geometry
> *geo)
> +{
> + /* some standard values */
> + geo->heads = 1 << 6;
> + geo->sectors = 1 << 5;
> + geo->cylinders = get_capacity(bd->bd_disk) >> 11;

Just stuffing the result of get_capacity into the 16-bit 
cylinders field will overflow/wrap on large capacities.
0x << 11 = 0x7FF_F800 = 64 GiB (68.7 GB)

How many programs still need these meaningless fields?
Could the bogus information be created elsewhere so
each block driver doesn't need to do this?


> + return 0;
> +}
> +
>  /*
>   * direct translation from (pmem,sector) => void*
>   * We do not require that sector be page aligned.
> @@ -176,6 +185,7 @@ out:
> 
>  static const struct block_device_operations pmem_fops = {
>   .owner =THIS_MODULE,
> + .getgeo =   pmem_getgeo,
>  };
> 
>  /* Kernel module stuff */
> --


---
Rob ElliottHP Server Storage



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH 2/4] pmem: Add support for getgeo()

2014-11-01 Thread Elliott, Robert (Server Storage)


 -Original Message-
 From: linux-kernel-ow...@vger.kernel.org [mailto:linux-kernel-
 ow...@vger.kernel.org] On Behalf Of Ross Zwisler
 Sent: Wednesday, 27 August, 2014 4:12 PM
 To: Jens Axboe; Matthew Wilcox; Boaz Harrosh; Nick Piggin; linux-
 fsde...@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
 nvd...@lists.01.org
 Cc: Ross Zwisler
 Subject: [PATCH 2/4] pmem: Add support for getgeo()
 
 Some programs require HDIO_GETGEO work, which requires we implement
 getgeo.  Based off of the work done to the NVMe driver in this
 commit:
 
 commit 4cc09e2dc4cb (NVMe: Add getgeo to block ops)
 
 Signed-off-by: Ross Zwisler ross.zwis...@linux.intel.com
 ---
  drivers/block/pmem.c | 10 ++
  1 file changed, 10 insertions(+)
 
 diff --git a/drivers/block/pmem.c b/drivers/block/pmem.c
 index d366b9b..60bbe0d 100644
 --- a/drivers/block/pmem.c
 +++ b/drivers/block/pmem.c
 @@ -50,6 +50,15 @@ struct pmem_device {
   size_t  size;
  };
 
 +static int pmem_getgeo(struct block_device *bd, struct hd_geometry
 *geo)
 +{
 + /* some standard values */
 + geo-heads = 1  6;
 + geo-sectors = 1  5;
 + geo-cylinders = get_capacity(bd-bd_disk)  11;

Just stuffing the result of get_capacity into the 16-bit 
cylinders field will overflow/wrap on large capacities.
0x  11 = 0x7FF_F800 = 64 GiB (68.7 GB)

How many programs still need these meaningless fields?
Could the bogus information be created elsewhere so
each block driver doesn't need to do this?


 + return 0;
 +}
 +
  /*
   * direct translation from (pmem,sector) = void*
   * We do not require that sector be page aligned.
 @@ -176,6 +185,7 @@ out:
 
  static const struct block_device_operations pmem_fops = {
   .owner =THIS_MODULE,
 + .getgeo =   pmem_getgeo,
  };
 
  /* Kernel module stuff */
 --


---
Rob ElliottHP Server Storage



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/