Re: [PATCH 01/15] lightnvm: simplify geometry structure.

2018-03-02 Thread Javier González 
> On 1 Mar 2018, at 11.22, Matias Bjørling  wrote:
> 
> On 02/28/2018 04:49 PM, Javier González wrote:
>> Currently, the device geometry is stored redundantly in the nvm_id and
>> nvm_geo structures at a device level. Moreover, when instantiating
>> targets on a specific number of LUNs, these structures are replicated
>> and manually modified to fit the instance channel and LUN partitioning.
>> Instead, create a generic geometry around nvm_geo, which can be used by
>> (i) the underlying device to describe the geometry of the whole device,
>> and (ii) instances to describe their geometry independently.
>> Signed-off-by: Javier González 
>> ---
>>  drivers/lightnvm/core.c  |  70 +++-
>>  drivers/lightnvm/pblk-core.c |  16 +-
>>  drivers/lightnvm/pblk-gc.c   |   2 +-
>>  drivers/lightnvm/pblk-init.c | 119 +++---
>>  drivers/lightnvm/pblk-read.c |   2 +-
>>  drivers/lightnvm/pblk-recovery.c |  14 +-
>>  drivers/lightnvm/pblk-rl.c   |   2 +-
>>  drivers/lightnvm/pblk-sysfs.c|  39 +++--
>>  drivers/lightnvm/pblk-write.c|   2 +-
>>  drivers/lightnvm/pblk.h  |  87 +-
>>  drivers/nvme/host/lightnvm.c | 341 
>> +++
>>  include/linux/lightnvm.h | 200 +++
>>  12 files changed, 465 insertions(+), 429 deletions(-)
>> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
>> index 19c46ebb1b91..9a417d9cdf0c 100644
>> --- a/drivers/lightnvm/core.c
>> +++ b/drivers/lightnvm/core.c
>> @@ -155,7 +155,7 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
>> nvm_dev *dev,
>>  int blun = lun_begin % dev->geo.nr_luns;
>>  int lunid = 0;
>>  int lun_balanced = 1;
>> -int prev_nr_luns;
>> +int sec_per_lun, prev_nr_luns;
>>  int i, j;
>>  nr_chnls = (nr_chnls_mod == 0) ? nr_chnls : nr_chnls + 1;
>> @@ -215,18 +215,23 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
>> nvm_dev *dev,
>>  if (!tgt_dev)
>>  goto err_ch;
>>  +   /* Inherit device geometry from parent */
>>  memcpy(_dev->geo, >geo, sizeof(struct nvm_geo));
>> +
>>  /* Target device only owns a portion of the physical device */
>>  tgt_dev->geo.nr_chnls = nr_chnls;
>> -tgt_dev->geo.all_luns = nr_luns;
>>  tgt_dev->geo.nr_luns = (lun_balanced) ? prev_nr_luns : -1;
>> +tgt_dev->geo.all_luns = nr_luns;
>> +tgt_dev->geo.all_chunks = nr_luns * dev->geo.nr_chks;
>> +
>>  tgt_dev->geo.op = op;
>> -tgt_dev->total_secs = nr_luns * tgt_dev->geo.sec_per_lun;
>> +
>> +sec_per_lun = dev->geo.clba * dev->geo.nr_chks;
>> +tgt_dev->geo.total_secs = nr_luns * sec_per_lun;
>> +
>>  tgt_dev->q = dev->q;
>>  tgt_dev->map = dev_map;
>>  tgt_dev->luns = luns;
>> -memcpy(_dev->identity, >identity, sizeof(struct nvm_id));
>> -
>>  tgt_dev->parent = dev;
>>  return tgt_dev;
>> @@ -296,8 +301,6 @@ static int __nvm_config_simple(struct nvm_dev *dev,
>>  static int __nvm_config_extended(struct nvm_dev *dev,
>>   struct nvm_ioctl_create_extended *e)
>>  {
>> -struct nvm_geo *geo = >geo;
>> -
>>  if (e->lun_begin == 0x && e->lun_end == 0x) {
>>  e->lun_begin = 0;
>>  e->lun_end = dev->geo.all_luns - 1;
>> @@ -311,7 +314,7 @@ static int __nvm_config_extended(struct nvm_dev *dev,
>>  return -EINVAL;
>>  }
>>  -   return nvm_config_check_luns(geo, e->lun_begin, e->lun_end);
>> +return nvm_config_check_luns(>geo, e->lun_begin, e->lun_end);
>>  }
>>static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create 
>> *create)
>> @@ -406,7 +409,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct 
>> nvm_ioctl_create *create)
>>  tqueue->queuedata = targetdata;
>>  blk_queue_max_hw_sectors(tqueue,
>> -(dev->geo.sec_size >> 9) * NVM_MAX_VLBA);
>> +(dev->geo.csecs >> 9) * NVM_MAX_VLBA);
>>  set_capacity(tdisk, tt->capacity(targetdata));
>>  add_disk(tdisk);
>> @@ -841,40 +844,9 @@ EXPORT_SYMBOL(nvm_get_tgt_bb_tbl);
>>static int nvm_core_init(struct nvm_dev *dev)
>>  {
>> -struct nvm_id *id = >identity;
>>  struct nvm_geo *geo = >geo;
>>  int ret;
>>  -   memcpy(>ppaf, >ppaf, sizeof(struct nvm_addr_format));
>> -
>> -if (id->mtype != 0) {
>> -pr_err("nvm: memory type not supported\n");
>> -return -EINVAL;
>> -}
>> -
>> -/* Whole device values */
>> -geo->nr_chnls = id->num_ch;
>> -geo->nr_luns = id->num_lun;
>> -
>> -/* Generic device geometry values */
>> -geo->ws_min = id->ws_min;
>> -geo->ws_opt = id->ws_opt;
>> -geo->ws_seq = id->ws_seq;
>> -geo->ws_per_chk = id->ws_per_chk;
>> -geo->nr_chks = id->num_chk;
>> -geo->mccap = id->mccap;
>> -
>> -geo->sec_per_chk = id->clba;
>> -geo->sec_per_lun = geo->sec_per_chk * geo->nr_chks;
>> -geo->all_luns =

Re: [PATCH 01/15] lightnvm: simplify geometry structure.

2018-03-01 Thread Matias Bjørling 

On 02/28/2018 04:49 PM, Javier González wrote:

Currently, the device geometry is stored redundantly in the nvm_id and
nvm_geo structures at a device level. Moreover, when instantiating
targets on a specific number of LUNs, these structures are replicated
and manually modified to fit the instance channel and LUN partitioning.

Instead, create a generic geometry around nvm_geo, which can be used by
(i) the underlying device to describe the geometry of the whole device,
and (ii) instances to describe their geometry independently.

Signed-off-by: Javier González 
---
  drivers/lightnvm/core.c  |  70 +++-
  drivers/lightnvm/pblk-core.c |  16 +-
  drivers/lightnvm/pblk-gc.c   |   2 +-
  drivers/lightnvm/pblk-init.c | 119 +++---
  drivers/lightnvm/pblk-read.c |   2 +-
  drivers/lightnvm/pblk-recovery.c |  14 +-
  drivers/lightnvm/pblk-rl.c   |   2 +-
  drivers/lightnvm/pblk-sysfs.c|  39 +++--
  drivers/lightnvm/pblk-write.c|   2 +-
  drivers/lightnvm/pblk.h  |  87 +-
  drivers/nvme/host/lightnvm.c | 341 +++
  include/linux/lightnvm.h | 200 +++
  12 files changed, 465 insertions(+), 429 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 19c46ebb1b91..9a417d9cdf0c 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -155,7 +155,7 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
int blun = lun_begin % dev->geo.nr_luns;
int lunid = 0;
int lun_balanced = 1;
-   int prev_nr_luns;
+   int sec_per_lun, prev_nr_luns;
int i, j;
  
  	nr_chnls = (nr_chnls_mod == 0) ? nr_chnls : nr_chnls + 1;

@@ -215,18 +215,23 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
if (!tgt_dev)
goto err_ch;
  
+	/* Inherit device geometry from parent */

memcpy(_dev->geo, >geo, sizeof(struct nvm_geo));
+
/* Target device only owns a portion of the physical device */
tgt_dev->geo.nr_chnls = nr_chnls;
-   tgt_dev->geo.all_luns = nr_luns;
tgt_dev->geo.nr_luns = (lun_balanced) ? prev_nr_luns : -1;
+   tgt_dev->geo.all_luns = nr_luns;
+   tgt_dev->geo.all_chunks = nr_luns * dev->geo.nr_chks;
+
tgt_dev->geo.op = op;
-   tgt_dev->total_secs = nr_luns * tgt_dev->geo.sec_per_lun;
+
+   sec_per_lun = dev->geo.clba * dev->geo.nr_chks;
+   tgt_dev->geo.total_secs = nr_luns * sec_per_lun;
+
tgt_dev->q = dev->q;
tgt_dev->map = dev_map;
tgt_dev->luns = luns;
-   memcpy(_dev->identity, >identity, sizeof(struct nvm_id));
-
tgt_dev->parent = dev;
  
  	return tgt_dev;

@@ -296,8 +301,6 @@ static int __nvm_config_simple(struct nvm_dev *dev,
  static int __nvm_config_extended(struct nvm_dev *dev,
 struct nvm_ioctl_create_extended *e)
  {
-   struct nvm_geo *geo = >geo;
-
if (e->lun_begin == 0x && e->lun_end == 0x) {
e->lun_begin = 0;
e->lun_end = dev->geo.all_luns - 1;
@@ -311,7 +314,7 @@ static int __nvm_config_extended(struct nvm_dev *dev,
return -EINVAL;
}
  
-	return nvm_config_check_luns(geo, e->lun_begin, e->lun_end);

+   return nvm_config_check_luns(>geo, e->lun_begin, e->lun_end);
  }
  
  static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)

@@ -406,7 +409,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct 
nvm_ioctl_create *create)
tqueue->queuedata = targetdata;
  
  	blk_queue_max_hw_sectors(tqueue,

-   (dev->geo.sec_size >> 9) * NVM_MAX_VLBA);
+   (dev->geo.csecs >> 9) * NVM_MAX_VLBA);
  
  	set_capacity(tdisk, tt->capacity(targetdata));

add_disk(tdisk);
@@ -841,40 +844,9 @@ EXPORT_SYMBOL(nvm_get_tgt_bb_tbl);
  
  static int nvm_core_init(struct nvm_dev *dev)

  {
-   struct nvm_id *id = >identity;
struct nvm_geo *geo = >geo;
int ret;
  
-	memcpy(>ppaf, >ppaf, sizeof(struct nvm_addr_format));

-
-   if (id->mtype != 0) {
-   pr_err("nvm: memory type not supported\n");
-   return -EINVAL;
-   }
-
-   /* Whole device values */
-   geo->nr_chnls = id->num_ch;
-   geo->nr_luns = id->num_lun;
-
-   /* Generic device geometry values */
-   geo->ws_min = id->ws_min;
-   geo->ws_opt = id->ws_opt;
-   geo->ws_seq = id->ws_seq;
-   geo->ws_per_chk = id->ws_per_chk;
-   geo->nr_chks = id->num_chk;
-   geo->mccap = id->mccap;
-
-   geo->sec_per_chk = id->clba;
-   geo->sec_per_lun = geo->sec_per_chk * geo->nr_chks;
-   geo->all_luns = geo->nr_luns * geo->nr_chnls;
-
-   /* 1.2 spec device geometry values */
-   geo->plane_mode = 1 << geo->ws_seq;
-   geo->nr_planes = geo->ws_opt / geo->ws_min;
-   geo->sec_per_pg = geo->ws_min;
-

[PATCH 01/15] lightnvm: simplify geometry structure.

2018-02-28 Thread Javier González 
Currently, the device geometry is stored redundantly in the nvm_id and
nvm_geo structures at a device level. Moreover, when instantiating
targets on a specific number of LUNs, these structures are replicated
and manually modified to fit the instance channel and LUN partitioning.

Instead, create a generic geometry around nvm_geo, which can be used by
(i) the underlying device to describe the geometry of the whole device,
and (ii) instances to describe their geometry independently.

Signed-off-by: Javier González 
---
 drivers/lightnvm/core.c  |  70 +++-
 drivers/lightnvm/pblk-core.c |  16 +-
 drivers/lightnvm/pblk-gc.c   |   2 +-
 drivers/lightnvm/pblk-init.c | 119 +++---
 drivers/lightnvm/pblk-read.c |   2 +-
 drivers/lightnvm/pblk-recovery.c |  14 +-
 drivers/lightnvm/pblk-rl.c   |   2 +-
 drivers/lightnvm/pblk-sysfs.c|  39 +++--
 drivers/lightnvm/pblk-write.c|   2 +-
 drivers/lightnvm/pblk.h  |  87 +-
 drivers/nvme/host/lightnvm.c | 341 +++
 include/linux/lightnvm.h | 200 +++
 12 files changed, 465 insertions(+), 429 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 19c46ebb1b91..9a417d9cdf0c 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -155,7 +155,7 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
int blun = lun_begin % dev->geo.nr_luns;
int lunid = 0;
int lun_balanced = 1;
-   int prev_nr_luns;
+   int sec_per_lun, prev_nr_luns;
int i, j;
 
nr_chnls = (nr_chnls_mod == 0) ? nr_chnls : nr_chnls + 1;
@@ -215,18 +215,23 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
if (!tgt_dev)
goto err_ch;
 
+   /* Inherit device geometry from parent */
memcpy(_dev->geo, >geo, sizeof(struct nvm_geo));
+
/* Target device only owns a portion of the physical device */
tgt_dev->geo.nr_chnls = nr_chnls;
-   tgt_dev->geo.all_luns = nr_luns;
tgt_dev->geo.nr_luns = (lun_balanced) ? prev_nr_luns : -1;
+   tgt_dev->geo.all_luns = nr_luns;
+   tgt_dev->geo.all_chunks = nr_luns * dev->geo.nr_chks;
+
tgt_dev->geo.op = op;
-   tgt_dev->total_secs = nr_luns * tgt_dev->geo.sec_per_lun;
+
+   sec_per_lun = dev->geo.clba * dev->geo.nr_chks;
+   tgt_dev->geo.total_secs = nr_luns * sec_per_lun;
+
tgt_dev->q = dev->q;
tgt_dev->map = dev_map;
tgt_dev->luns = luns;
-   memcpy(_dev->identity, >identity, sizeof(struct nvm_id));
-
tgt_dev->parent = dev;
 
return tgt_dev;
@@ -296,8 +301,6 @@ static int __nvm_config_simple(struct nvm_dev *dev,
 static int __nvm_config_extended(struct nvm_dev *dev,
 struct nvm_ioctl_create_extended *e)
 {
-   struct nvm_geo *geo = >geo;
-
if (e->lun_begin == 0x && e->lun_end == 0x) {
e->lun_begin = 0;
e->lun_end = dev->geo.all_luns - 1;
@@ -311,7 +314,7 @@ static int __nvm_config_extended(struct nvm_dev *dev,
return -EINVAL;
}
 
-   return nvm_config_check_luns(geo, e->lun_begin, e->lun_end);
+   return nvm_config_check_luns(>geo, e->lun_begin, e->lun_end);
 }
 
 static int nvm_create_tgt(struct nvm_dev *dev, struct nvm_ioctl_create *create)
@@ -406,7 +409,7 @@ static int nvm_create_tgt(struct nvm_dev *dev, struct 
nvm_ioctl_create *create)
tqueue->queuedata = targetdata;
 
blk_queue_max_hw_sectors(tqueue,
-   (dev->geo.sec_size >> 9) * NVM_MAX_VLBA);
+   (dev->geo.csecs >> 9) * NVM_MAX_VLBA);
 
set_capacity(tdisk, tt->capacity(targetdata));
add_disk(tdisk);
@@ -841,40 +844,9 @@ EXPORT_SYMBOL(nvm_get_tgt_bb_tbl);
 
 static int nvm_core_init(struct nvm_dev *dev)
 {
-   struct nvm_id *id = >identity;
struct nvm_geo *geo = >geo;
int ret;
 
-   memcpy(>ppaf, >ppaf, sizeof(struct nvm_addr_format));
-
-   if (id->mtype != 0) {
-   pr_err("nvm: memory type not supported\n");
-   return -EINVAL;
-   }
-
-   /* Whole device values */
-   geo->nr_chnls = id->num_ch;
-   geo->nr_luns = id->num_lun;
-
-   /* Generic device geometry values */
-   geo->ws_min = id->ws_min;
-   geo->ws_opt = id->ws_opt;
-   geo->ws_seq = id->ws_seq;
-   geo->ws_per_chk = id->ws_per_chk;
-   geo->nr_chks = id->num_chk;
-   geo->mccap = id->mccap;
-
-   geo->sec_per_chk = id->clba;
-   geo->sec_per_lun = geo->sec_per_chk * geo->nr_chks;
-   geo->all_luns = geo->nr_luns * geo->nr_chnls;
-
-   /* 1.2 spec device geometry values */
-   geo->plane_mode = 1 << geo->ws_seq;
-   geo->nr_planes = geo->ws_opt / geo->ws_min;
-   geo->sec_per_pg = geo->ws_min;
-   geo->sec_per_pl = geo->sec_per_pg * geo->nr_planes;