Re: [PATCH 1/8] lightnvm: exposed generic geometry to targets

[Javier Gonzalez]

> On 15 Feb 2018, at 02.13, Matias Bjørling  wrote:
> 
>> On 02/13/2018 03:06 PM, Javier González wrote:
>> With the inclusion of 2.0 support, we need a generic geometry that
>> describes the OCSSD independently of the specification that it
>> implements. Otherwise, geometry specific code is required, which
>> complicates targets and makes maintenance much more difficult.
>> This patch refactors the identify path and populates a generic geometry
>> that is then given to the targets on creation. Since the 2.0 geometry is
>> much more abstract that 1.2, the generic geometry resembles 2.0, but it
>> is not identical, as it needs to understand 1.2 abstractions too.
>> Signed-off-by: Javier González 
>> ---
>>  drivers/lightnvm/core.c  | 143 ++-
>>  drivers/lightnvm/pblk-core.c |  16 +-
>>  drivers/lightnvm/pblk-gc.c   |   2 +-
>>  drivers/lightnvm/pblk-init.c | 149 ---
>>  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  | 105 +--
>>  drivers/nvme/host/lightnvm.c | 379 
>> ---
>>  include/linux/lightnvm.h | 220 +--
>>  12 files changed, 586 insertions(+), 487 deletions(-)
>> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
>> index 9b1255b3e05e..80492fa6ee76 100644
>> --- a/drivers/lightnvm/core.c
>> +++ b/drivers/lightnvm/core.c
>> @@ -111,6 +111,7 @@ static void nvm_release_luns_err(struct nvm_dev *dev, 
>> int lun_begin,
>>  static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear)
>>  {
>>  struct nvm_dev *dev = tgt_dev->parent;
>> +struct nvm_dev_geo *dev_geo = >dev_geo;
>>  struct nvm_dev_map *dev_map = tgt_dev->map;
>>  int i, j;
>>  @@ -122,7 +123,7 @@ static void nvm_remove_tgt_dev(struct nvm_tgt_dev 
>> *tgt_dev, int clear)
>>  if (clear) {
>>  for (j = 0; j < ch_map->nr_luns; j++) {
>>  int lun = j + lun_offs[j];
>> -int lunid = (ch * dev->geo.nr_luns) + lun;
>> +int lunid = (ch * dev_geo->num_lun) + lun;
>>WARN_ON(!test_and_clear_bit(lunid,
>>  dev->lun_map));
>> @@ -143,19 +144,20 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
>> nvm_dev *dev,
>>u16 lun_begin, u16 lun_end,
>>u16 op)
>>  {
>> +struct nvm_dev_geo *dev_geo = >dev_geo;
>>  struct nvm_tgt_dev *tgt_dev = NULL;
>>  struct nvm_dev_map *dev_rmap = dev->rmap;
>>  struct nvm_dev_map *dev_map;
>>  struct ppa_addr *luns;
>>  int nr_luns = lun_end - lun_begin + 1;
>>  int luns_left = nr_luns;
>> -int nr_chnls = nr_luns / dev->geo.nr_luns;
>> -int nr_chnls_mod = nr_luns % dev->geo.nr_luns;
>> -int bch = lun_begin / dev->geo.nr_luns;
>> -int blun = lun_begin % dev->geo.nr_luns;
>> +int nr_chnls = nr_luns / dev_geo->num_lun;
>> +int nr_chnls_mod = nr_luns % dev_geo->num_lun;
>> +int bch = lun_begin / dev_geo->num_lun;
>> +int blun = lun_begin % dev_geo->num_lun;
>>  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;
>> @@ -173,15 +175,15 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
>> nvm_dev *dev,
>>  if (!luns)
>>  goto err_luns;
>>  -prev_nr_luns = (luns_left > dev->geo.nr_luns) ?
>> -dev->geo.nr_luns : luns_left;
>> +prev_nr_luns = (luns_left > dev_geo->num_lun) ?
>> +dev_geo->num_lun : luns_left;
>>  for (i = 0; i < nr_chnls; i++) {
>>  struct nvm_ch_map *ch_rmap = _rmap->chnls[i + bch];
>>  int *lun_roffs = ch_rmap->lun_offs;
>>  struct nvm_ch_map *ch_map = _map->chnls[i];
>>  int *lun_offs;
>> -int luns_in_chnl = (luns_left > dev->geo.nr_luns) ?
>> -dev->geo.nr_luns : luns_left;
>> +int luns_in_chnl = (luns_left > dev_geo->num_lun) ?
>> +dev_geo->num_lun : luns_left;
>>if (lun_balanced && prev_nr_luns != luns_in_chnl)
>>  lun_balanced = 0;
>> @@ -215,18 +217,23 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
>> nvm_dev *dev,
>>  if (!tgt_dev)
>>  goto err_ch;
>>  -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.num_ch = nr_chnls;
>> +tgt_dev->geo.num_lun = (lun_balanced) ? prev_nr_luns : -1;
>>  tgt_dev->geo.all_luns = nr_luns;
>> -tgt_dev->geo.nr_luns = (lun_balanced) ? prev_nr_luns : -1;
>> 

Re: [PATCH 1/8] lightnvm: exposed generic geometry to targets

[Javier Gonzalez]

> On 15 Feb 2018, at 02.13, Matias Bjørling  wrote:
> 
>> On 02/13/2018 03:06 PM, Javier González wrote:
>> With the inclusion of 2.0 support, we need a generic geometry that
>> describes the OCSSD independently of the specification that it
>> implements. Otherwise, geometry specific code is required, which
>> complicates targets and makes maintenance much more difficult.
>> This patch refactors the identify path and populates a generic geometry
>> that is then given to the targets on creation. Since the 2.0 geometry is
>> much more abstract that 1.2, the generic geometry resembles 2.0, but it
>> is not identical, as it needs to understand 1.2 abstractions too.
>> Signed-off-by: Javier González 
>> ---
>>  drivers/lightnvm/core.c  | 143 ++-
>>  drivers/lightnvm/pblk-core.c |  16 +-
>>  drivers/lightnvm/pblk-gc.c   |   2 +-
>>  drivers/lightnvm/pblk-init.c | 149 ---
>>  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  | 105 +--
>>  drivers/nvme/host/lightnvm.c | 379 
>> ---
>>  include/linux/lightnvm.h | 220 +--
>>  12 files changed, 586 insertions(+), 487 deletions(-)
>> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
>> index 9b1255b3e05e..80492fa6ee76 100644
>> --- a/drivers/lightnvm/core.c
>> +++ b/drivers/lightnvm/core.c
>> @@ -111,6 +111,7 @@ static void nvm_release_luns_err(struct nvm_dev *dev, 
>> int lun_begin,
>>  static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear)
>>  {
>>  struct nvm_dev *dev = tgt_dev->parent;
>> +struct nvm_dev_geo *dev_geo = >dev_geo;
>>  struct nvm_dev_map *dev_map = tgt_dev->map;
>>  int i, j;
>>  @@ -122,7 +123,7 @@ static void nvm_remove_tgt_dev(struct nvm_tgt_dev 
>> *tgt_dev, int clear)
>>  if (clear) {
>>  for (j = 0; j < ch_map->nr_luns; j++) {
>>  int lun = j + lun_offs[j];
>> -int lunid = (ch * dev->geo.nr_luns) + lun;
>> +int lunid = (ch * dev_geo->num_lun) + lun;
>>WARN_ON(!test_and_clear_bit(lunid,
>>  dev->lun_map));
>> @@ -143,19 +144,20 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
>> nvm_dev *dev,
>>u16 lun_begin, u16 lun_end,
>>u16 op)
>>  {
>> +struct nvm_dev_geo *dev_geo = >dev_geo;
>>  struct nvm_tgt_dev *tgt_dev = NULL;
>>  struct nvm_dev_map *dev_rmap = dev->rmap;
>>  struct nvm_dev_map *dev_map;
>>  struct ppa_addr *luns;
>>  int nr_luns = lun_end - lun_begin + 1;
>>  int luns_left = nr_luns;
>> -int nr_chnls = nr_luns / dev->geo.nr_luns;
>> -int nr_chnls_mod = nr_luns % dev->geo.nr_luns;
>> -int bch = lun_begin / dev->geo.nr_luns;
>> -int blun = lun_begin % dev->geo.nr_luns;
>> +int nr_chnls = nr_luns / dev_geo->num_lun;
>> +int nr_chnls_mod = nr_luns % dev_geo->num_lun;
>> +int bch = lun_begin / dev_geo->num_lun;
>> +int blun = lun_begin % dev_geo->num_lun;
>>  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;
>> @@ -173,15 +175,15 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
>> nvm_dev *dev,
>>  if (!luns)
>>  goto err_luns;
>>  -prev_nr_luns = (luns_left > dev->geo.nr_luns) ?
>> -dev->geo.nr_luns : luns_left;
>> +prev_nr_luns = (luns_left > dev_geo->num_lun) ?
>> +dev_geo->num_lun : luns_left;
>>  for (i = 0; i < nr_chnls; i++) {
>>  struct nvm_ch_map *ch_rmap = _rmap->chnls[i + bch];
>>  int *lun_roffs = ch_rmap->lun_offs;
>>  struct nvm_ch_map *ch_map = _map->chnls[i];
>>  int *lun_offs;
>> -int luns_in_chnl = (luns_left > dev->geo.nr_luns) ?
>> -dev->geo.nr_luns : luns_left;
>> +int luns_in_chnl = (luns_left > dev_geo->num_lun) ?
>> +dev_geo->num_lun : luns_left;
>>if (lun_balanced && prev_nr_luns != luns_in_chnl)
>>  lun_balanced = 0;
>> @@ -215,18 +217,23 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
>> nvm_dev *dev,
>>  if (!tgt_dev)
>>  goto err_ch;
>>  -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.num_ch = nr_chnls;
>> +tgt_dev->geo.num_lun = (lun_balanced) ? prev_nr_luns : -1;
>>  tgt_dev->geo.all_luns = nr_luns;
>> -tgt_dev->geo.nr_luns = (lun_balanced) ? prev_nr_luns : -1;
>> +tgt_dev->geo.all_chunks = nr_luns 

Re: [PATCH 1/8] lightnvm: exposed generic geometry to targets

[Matias Bjørling]

On 02/13/2018 03:06 PM, Javier González wrote:

With the inclusion of 2.0 support, we need a generic geometry that
describes the OCSSD independently of the specification that it
implements. Otherwise, geometry specific code is required, which
complicates targets and makes maintenance much more difficult.

This patch refactors the identify path and populates a generic geometry
that is then given to the targets on creation. Since the 2.0 geometry is
much more abstract that 1.2, the generic geometry resembles 2.0, but it
is not identical, as it needs to understand 1.2 abstractions too.

Signed-off-by: Javier González 
---
  drivers/lightnvm/core.c  | 143 ++-
  drivers/lightnvm/pblk-core.c |  16 +-
  drivers/lightnvm/pblk-gc.c   |   2 +-
  drivers/lightnvm/pblk-init.c | 149 ---
  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  | 105 +--
  drivers/nvme/host/lightnvm.c | 379 ---
  include/linux/lightnvm.h | 220 +--
  12 files changed, 586 insertions(+), 487 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 9b1255b3e05e..80492fa6ee76 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -111,6 +111,7 @@ static void nvm_release_luns_err(struct nvm_dev *dev, int 
lun_begin,
  static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear)
  {
struct nvm_dev *dev = tgt_dev->parent;
+   struct nvm_dev_geo *dev_geo = >dev_geo;
struct nvm_dev_map *dev_map = tgt_dev->map;
int i, j;
  
@@ -122,7 +123,7 @@ static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear)

if (clear) {
for (j = 0; j < ch_map->nr_luns; j++) {
int lun = j + lun_offs[j];
-   int lunid = (ch * dev->geo.nr_luns) + lun;
+   int lunid = (ch * dev_geo->num_lun) + lun;
  
  WARN_ON(!test_and_clear_bit(lunid,

dev->lun_map));
@@ -143,19 +144,20 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
  u16 lun_begin, u16 lun_end,
  u16 op)
  {
+   struct nvm_dev_geo *dev_geo = >dev_geo;
struct nvm_tgt_dev *tgt_dev = NULL;
struct nvm_dev_map *dev_rmap = dev->rmap;
struct nvm_dev_map *dev_map;
struct ppa_addr *luns;
int nr_luns = lun_end - lun_begin + 1;
int luns_left = nr_luns;
-   int nr_chnls = nr_luns / dev->geo.nr_luns;
-   int nr_chnls_mod = nr_luns % dev->geo.nr_luns;
-   int bch = lun_begin / dev->geo.nr_luns;
-   int blun = lun_begin % dev->geo.nr_luns;
+   int nr_chnls = nr_luns / dev_geo->num_lun;
+   int nr_chnls_mod = nr_luns % dev_geo->num_lun;
+   int bch = lun_begin / dev_geo->num_lun;
+   int blun = lun_begin % dev_geo->num_lun;
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;

@@ -173,15 +175,15 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
if (!luns)
goto err_luns;
  
-	prev_nr_luns = (luns_left > dev->geo.nr_luns) ?

-   dev->geo.nr_luns : luns_left;
+   prev_nr_luns = (luns_left > dev_geo->num_lun) ?
+   dev_geo->num_lun : luns_left;
for (i = 0; i < nr_chnls; i++) {
struct nvm_ch_map *ch_rmap = _rmap->chnls[i + bch];
int *lun_roffs = ch_rmap->lun_offs;
struct nvm_ch_map *ch_map = _map->chnls[i];
int *lun_offs;
-   int luns_in_chnl = (luns_left > dev->geo.nr_luns) ?
-   dev->geo.nr_luns : luns_left;
+   int luns_in_chnl = (luns_left > dev_geo->num_lun) ?
+   dev_geo->num_lun : luns_left;
  
  		if (lun_balanced && prev_nr_luns != luns_in_chnl)

lun_balanced = 0;
@@ -215,18 +217,23 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
if (!tgt_dev)
goto err_ch;
  
-	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.num_ch = nr_chnls;
+   tgt_dev->geo.num_lun = (lun_balanced) ? prev_nr_luns : -1;
tgt_dev->geo.all_luns = nr_luns;
-   tgt_dev->geo.nr_luns = (lun_balanced) ? prev_nr_luns : -1;

Re: [PATCH 1/8] lightnvm: exposed generic geometry to targets

[Matias Bjørling]

On 02/13/2018 03:06 PM, Javier González wrote:

With the inclusion of 2.0 support, we need a generic geometry that
describes the OCSSD independently of the specification that it
implements. Otherwise, geometry specific code is required, which
complicates targets and makes maintenance much more difficult.

This patch refactors the identify path and populates a generic geometry
that is then given to the targets on creation. Since the 2.0 geometry is
much more abstract that 1.2, the generic geometry resembles 2.0, but it
is not identical, as it needs to understand 1.2 abstractions too.

Signed-off-by: Javier González 
---
  drivers/lightnvm/core.c  | 143 ++-
  drivers/lightnvm/pblk-core.c |  16 +-
  drivers/lightnvm/pblk-gc.c   |   2 +-
  drivers/lightnvm/pblk-init.c | 149 ---
  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  | 105 +--
  drivers/nvme/host/lightnvm.c | 379 ---
  include/linux/lightnvm.h | 220 +--
  12 files changed, 586 insertions(+), 487 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index 9b1255b3e05e..80492fa6ee76 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -111,6 +111,7 @@ static void nvm_release_luns_err(struct nvm_dev *dev, int 
lun_begin,
  static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear)
  {
struct nvm_dev *dev = tgt_dev->parent;
+   struct nvm_dev_geo *dev_geo = >dev_geo;
struct nvm_dev_map *dev_map = tgt_dev->map;
int i, j;
  
@@ -122,7 +123,7 @@ static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear)

if (clear) {
for (j = 0; j < ch_map->nr_luns; j++) {
int lun = j + lun_offs[j];
-   int lunid = (ch * dev->geo.nr_luns) + lun;
+   int lunid = (ch * dev_geo->num_lun) + lun;
  
  WARN_ON(!test_and_clear_bit(lunid,

dev->lun_map));
@@ -143,19 +144,20 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
  u16 lun_begin, u16 lun_end,
  u16 op)
  {
+   struct nvm_dev_geo *dev_geo = >dev_geo;
struct nvm_tgt_dev *tgt_dev = NULL;
struct nvm_dev_map *dev_rmap = dev->rmap;
struct nvm_dev_map *dev_map;
struct ppa_addr *luns;
int nr_luns = lun_end - lun_begin + 1;
int luns_left = nr_luns;
-   int nr_chnls = nr_luns / dev->geo.nr_luns;
-   int nr_chnls_mod = nr_luns % dev->geo.nr_luns;
-   int bch = lun_begin / dev->geo.nr_luns;
-   int blun = lun_begin % dev->geo.nr_luns;
+   int nr_chnls = nr_luns / dev_geo->num_lun;
+   int nr_chnls_mod = nr_luns % dev_geo->num_lun;
+   int bch = lun_begin / dev_geo->num_lun;
+   int blun = lun_begin % dev_geo->num_lun;
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;

@@ -173,15 +175,15 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
if (!luns)
goto err_luns;
  
-	prev_nr_luns = (luns_left > dev->geo.nr_luns) ?

-   dev->geo.nr_luns : luns_left;
+   prev_nr_luns = (luns_left > dev_geo->num_lun) ?
+   dev_geo->num_lun : luns_left;
for (i = 0; i < nr_chnls; i++) {
struct nvm_ch_map *ch_rmap = _rmap->chnls[i + bch];
int *lun_roffs = ch_rmap->lun_offs;
struct nvm_ch_map *ch_map = _map->chnls[i];
int *lun_offs;
-   int luns_in_chnl = (luns_left > dev->geo.nr_luns) ?
-   dev->geo.nr_luns : luns_left;
+   int luns_in_chnl = (luns_left > dev_geo->num_lun) ?
+   dev_geo->num_lun : luns_left;
  
  		if (lun_balanced && prev_nr_luns != luns_in_chnl)

lun_balanced = 0;
@@ -215,18 +217,23 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct 
nvm_dev *dev,
if (!tgt_dev)
goto err_ch;
  
-	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.num_ch = nr_chnls;
+   tgt_dev->geo.num_lun = (lun_balanced) ? prev_nr_luns : -1;
tgt_dev->geo.all_luns = nr_luns;
-   tgt_dev->geo.nr_luns = (lun_balanced) ? prev_nr_luns : -1;
+