Re: [PATCH v2] lightnvm: divide global reverse translation map into per lun

2016-03-22 Thread Wenwei Tao 
Seems it is the variable pg_offset in rrpc_page_invalidate that cause the panic.
It's a u64 variable we use its low 32bit to store the offset value,
leave the high 32bit in a undetermined status, this will cause
test_and_set_bit access unexpected memory, cause kernel panic.
It's should be initialized to zero before use.

I add some log  to verify this assuption, bleow is the log

[  485.260549] rrpc: paddr: 400 sec_per_blk: 100 offset: 8800
[  485.260563] BUG: unable to handle kernel paging request at bad4f230

offset value 8800 may be passed to test_and_set_bit instead of 0.

2016-03-21 18:25 GMT+08:00 Matias Bjørling :
> On 03/20/2016 06:51 AM, Wenwei Tao wrote:
>>
>> Divide the target's global reverse translation map into per lun,
>> to prepare support for the non-continuous lun target creation.
>>
>> Signed-off-by: Wenwei Tao 
>> ---
>>
>> Changes since v1
>> -fix merge/rebase mistake in rrpc_block_map_update().
>> -remove variables poffset and lun_offset in rrpc structure
>> since no one use them now.
>>
>>   drivers/lightnvm/rrpc.c  | 181
>> +++
>>   drivers/lightnvm/rrpc.h  |   9 ++-
>>   include/linux/lightnvm.h |   1 +
>>   3 files changed, 111 insertions(+), 80 deletions(-)
>>
>> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
>> index 3ab6495..88e2ce5 100644
>> --- a/drivers/lightnvm/rrpc.c
>> +++ b/drivers/lightnvm/rrpc.c
>> @@ -23,28 +23,35 @@ static int rrpc_submit_io(struct rrpc *rrpc, struct
>> bio *bio,
>> struct nvm_rq *rqd, unsigned long flags);
>>
>>   #define rrpc_for_each_lun(rrpc, rlun, i) \
>> -   for ((i) = 0, rlun = &(rrpc)->luns[0]; \
>> -   (i) < (rrpc)->nr_luns; (i)++, rlun =
>> &(rrpc)->luns[(i)])
>> +   for ((i) = 0, rlun = &(rrpc)->luns[0]; \
>> +   (i) < (rrpc)->nr_luns; (i)++, rlun = &(rrpc)->luns[(i)])
>> +
>> +static inline u64 lun_poffset(struct nvm_lun *lun, struct nvm_dev *dev)
>> +{
>> +   return lun->id * dev->sec_per_lun;
>> +}
>>
>>   static void rrpc_page_invalidate(struct rrpc *rrpc, struct rrpc_addr *a)
>>   {
>> struct rrpc_block *rblk = a->rblk;
>> -   unsigned int pg_offset;
>> +   struct rrpc_lun *rlun = rblk->rlun;
>> +   u64 pg_offset;
>>
>> -   lockdep_assert_held(>rev_lock);
>> +   lockdep_assert_held(>rev_lock);
>>
>> if (a->addr == ADDR_EMPTY || !rblk)
>> return;
>>
>> spin_lock(>lock);
>>
>> -   div_u64_rem(a->addr, rrpc->dev->sec_per_blk, _offset);
>> +   div_u64_rem(a->addr, rrpc->dev->sec_per_blk, (u32 *)_offset);
>> WARN_ON(test_and_set_bit(pg_offset, rblk->invalid_pages));
>> rblk->nr_invalid_pages++;
>>
>> spin_unlock(>lock);
>>
>> -   rrpc->rev_trans_map[a->addr - rrpc->poffset].addr = ADDR_EMPTY;
>> +   pg_offset = lun_poffset(rlun->parent, rrpc->dev);
>> +   rlun->rev_trans_map[a->addr - pg_offset].addr = ADDR_EMPTY;
>>   }
>>
>>   static void rrpc_invalidate_range(struct rrpc *rrpc, sector_t slba,
>> @@ -52,14 +59,15 @@ static void rrpc_invalidate_range(struct rrpc *rrpc,
>> sector_t slba,
>>   {
>> sector_t i;
>>
>> -   spin_lock(>rev_lock);
>> for (i = slba; i < slba + len; i++) {
>> struct rrpc_addr *gp = >trans_map[i];
>> +   struct rrpc_lun *rlun = gp->rblk->rlun;
>>
>> +   spin_lock(>rev_lock);
>> rrpc_page_invalidate(rrpc, gp);
>> +   spin_unlock(>rev_lock);
>> gp->rblk = NULL;
>> }
>> -   spin_unlock(>rev_lock);
>>   }
>>
>>   static struct nvm_rq *rrpc_inflight_laddr_acquire(struct rrpc *rrpc,
>> @@ -116,15 +124,6 @@ static int block_is_full(struct rrpc *rrpc, struct
>> rrpc_block *rblk)
>> return (rblk->next_page == rrpc->dev->sec_per_blk);
>>   }
>>
>> -/* Calculate relative addr for the given block, considering instantiated
>> LUNs */
>> -static u64 block_to_rel_addr(struct rrpc *rrpc, struct rrpc_block *rblk)
>> -{
>> -   struct nvm_block *blk = rblk->parent;
>> -   int lun_blk = blk->id % (rrpc->dev->blks_per_lun * rrpc->nr_luns);
>> -
>> -   return lun_blk * rrpc->dev->sec_per_blk;
>> -}
>> -
>>   /* Calculate global addr for the given block */
>>   static u64 block_to_addr(struct rrpc *rrpc, struct rrpc_block *rblk)
>>   {
>> @@ -291,13 +290,14 @@ static void rrpc_end_sync_bio(struct bio *bio)
>>   static int rrpc_move_valid_pages(struct rrpc *rrpc, struct rrpc_block
>> *rblk)
>>   {
>> struct request_queue *q = rrpc->dev->q;
>> +   struct rrpc_lun *rlun = rblk->rlun;
>> struct rrpc_rev_addr *rev;
>> struct nvm_rq *rqd;
>> struct bio *bio;
>> struct page *page;
>> int slot;
>> int nr_sec_per_blk = rrpc->dev->sec_per_blk;
>> -   u64 phys_addr;
>> +   u64 phys_addr, poffset;
>>

Re: [PATCH v2] lightnvm: divide global reverse translation map into per lun

2016-03-22 Thread Wenwei Tao 
Seems it is the variable pg_offset in rrpc_page_invalidate that cause the panic.
It's a u64 variable we use its low 32bit to store the offset value,
leave the high 32bit in a undetermined status, this will cause
test_and_set_bit access unexpected memory, cause kernel panic.
It's should be initialized to zero before use.

I add some log  to verify this assuption, bleow is the log

[  485.260549] rrpc: paddr: 400 sec_per_blk: 100 offset: 8800
[  485.260563] BUG: unable to handle kernel paging request at bad4f230

offset value 8800 may be passed to test_and_set_bit instead of 0.

2016-03-21 18:25 GMT+08:00 Matias Bjørling :
> On 03/20/2016 06:51 AM, Wenwei Tao wrote:
>>
>> Divide the target's global reverse translation map into per lun,
>> to prepare support for the non-continuous lun target creation.
>>
>> Signed-off-by: Wenwei Tao 
>> ---
>>
>> Changes since v1
>> -fix merge/rebase mistake in rrpc_block_map_update().
>> -remove variables poffset and lun_offset in rrpc structure
>> since no one use them now.
>>
>>   drivers/lightnvm/rrpc.c  | 181
>> +++
>>   drivers/lightnvm/rrpc.h  |   9 ++-
>>   include/linux/lightnvm.h |   1 +
>>   3 files changed, 111 insertions(+), 80 deletions(-)
>>
>> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
>> index 3ab6495..88e2ce5 100644
>> --- a/drivers/lightnvm/rrpc.c
>> +++ b/drivers/lightnvm/rrpc.c
>> @@ -23,28 +23,35 @@ static int rrpc_submit_io(struct rrpc *rrpc, struct
>> bio *bio,
>> struct nvm_rq *rqd, unsigned long flags);
>>
>>   #define rrpc_for_each_lun(rrpc, rlun, i) \
>> -   for ((i) = 0, rlun = &(rrpc)->luns[0]; \
>> -   (i) < (rrpc)->nr_luns; (i)++, rlun =
>> &(rrpc)->luns[(i)])
>> +   for ((i) = 0, rlun = &(rrpc)->luns[0]; \
>> +   (i) < (rrpc)->nr_luns; (i)++, rlun = &(rrpc)->luns[(i)])
>> +
>> +static inline u64 lun_poffset(struct nvm_lun *lun, struct nvm_dev *dev)
>> +{
>> +   return lun->id * dev->sec_per_lun;
>> +}
>>
>>   static void rrpc_page_invalidate(struct rrpc *rrpc, struct rrpc_addr *a)
>>   {
>> struct rrpc_block *rblk = a->rblk;
>> -   unsigned int pg_offset;
>> +   struct rrpc_lun *rlun = rblk->rlun;
>> +   u64 pg_offset;
>>
>> -   lockdep_assert_held(>rev_lock);
>> +   lockdep_assert_held(>rev_lock);
>>
>> if (a->addr == ADDR_EMPTY || !rblk)
>> return;
>>
>> spin_lock(>lock);
>>
>> -   div_u64_rem(a->addr, rrpc->dev->sec_per_blk, _offset);
>> +   div_u64_rem(a->addr, rrpc->dev->sec_per_blk, (u32 *)_offset);
>> WARN_ON(test_and_set_bit(pg_offset, rblk->invalid_pages));
>> rblk->nr_invalid_pages++;
>>
>> spin_unlock(>lock);
>>
>> -   rrpc->rev_trans_map[a->addr - rrpc->poffset].addr = ADDR_EMPTY;
>> +   pg_offset = lun_poffset(rlun->parent, rrpc->dev);
>> +   rlun->rev_trans_map[a->addr - pg_offset].addr = ADDR_EMPTY;
>>   }
>>
>>   static void rrpc_invalidate_range(struct rrpc *rrpc, sector_t slba,
>> @@ -52,14 +59,15 @@ static void rrpc_invalidate_range(struct rrpc *rrpc,
>> sector_t slba,
>>   {
>> sector_t i;
>>
>> -   spin_lock(>rev_lock);
>> for (i = slba; i < slba + len; i++) {
>> struct rrpc_addr *gp = >trans_map[i];
>> +   struct rrpc_lun *rlun = gp->rblk->rlun;
>>
>> +   spin_lock(>rev_lock);
>> rrpc_page_invalidate(rrpc, gp);
>> +   spin_unlock(>rev_lock);
>> gp->rblk = NULL;
>> }
>> -   spin_unlock(>rev_lock);
>>   }
>>
>>   static struct nvm_rq *rrpc_inflight_laddr_acquire(struct rrpc *rrpc,
>> @@ -116,15 +124,6 @@ static int block_is_full(struct rrpc *rrpc, struct
>> rrpc_block *rblk)
>> return (rblk->next_page == rrpc->dev->sec_per_blk);
>>   }
>>
>> -/* Calculate relative addr for the given block, considering instantiated
>> LUNs */
>> -static u64 block_to_rel_addr(struct rrpc *rrpc, struct rrpc_block *rblk)
>> -{
>> -   struct nvm_block *blk = rblk->parent;
>> -   int lun_blk = blk->id % (rrpc->dev->blks_per_lun * rrpc->nr_luns);
>> -
>> -   return lun_blk * rrpc->dev->sec_per_blk;
>> -}
>> -
>>   /* Calculate global addr for the given block */
>>   static u64 block_to_addr(struct rrpc *rrpc, struct rrpc_block *rblk)
>>   {
>> @@ -291,13 +290,14 @@ static void rrpc_end_sync_bio(struct bio *bio)
>>   static int rrpc_move_valid_pages(struct rrpc *rrpc, struct rrpc_block
>> *rblk)
>>   {
>> struct request_queue *q = rrpc->dev->q;
>> +   struct rrpc_lun *rlun = rblk->rlun;
>> struct rrpc_rev_addr *rev;
>> struct nvm_rq *rqd;
>> struct bio *bio;
>> struct page *page;
>> int slot;
>> int nr_sec_per_blk = rrpc->dev->sec_per_blk;
>> -   u64 phys_addr;
>> +   u64 phys_addr, poffset;
>> DECLARE_COMPLETION_ONSTACK(wait);
>>
>> if

Re: [PATCH v2] lightnvm: divide global reverse translation map into per lun

2016-03-21 Thread Matias Bjørling 

On 03/20/2016 06:51 AM, Wenwei Tao wrote:

Divide the target's global reverse translation map into per lun,
to prepare support for the non-continuous lun target creation.

Signed-off-by: Wenwei Tao 
---

Changes since v1
-fix merge/rebase mistake in rrpc_block_map_update().
-remove variables poffset and lun_offset in rrpc structure
since no one use them now.

  drivers/lightnvm/rrpc.c  | 181 +++
  drivers/lightnvm/rrpc.h  |   9 ++-
  include/linux/lightnvm.h |   1 +
  3 files changed, 111 insertions(+), 80 deletions(-)

diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
index 3ab6495..88e2ce5 100644
--- a/drivers/lightnvm/rrpc.c
+++ b/drivers/lightnvm/rrpc.c
@@ -23,28 +23,35 @@ static int rrpc_submit_io(struct rrpc *rrpc, struct bio 
*bio,
struct nvm_rq *rqd, unsigned long flags);

  #define rrpc_for_each_lun(rrpc, rlun, i) \
-   for ((i) = 0, rlun = &(rrpc)->luns[0]; \
-   (i) < (rrpc)->nr_luns; (i)++, rlun = &(rrpc)->luns[(i)])
+   for ((i) = 0, rlun = &(rrpc)->luns[0]; \
+   (i) < (rrpc)->nr_luns; (i)++, rlun = &(rrpc)->luns[(i)])
+
+static inline u64 lun_poffset(struct nvm_lun *lun, struct nvm_dev *dev)
+{
+   return lun->id * dev->sec_per_lun;
+}

  static void rrpc_page_invalidate(struct rrpc *rrpc, struct rrpc_addr *a)
  {
struct rrpc_block *rblk = a->rblk;
-   unsigned int pg_offset;
+   struct rrpc_lun *rlun = rblk->rlun;
+   u64 pg_offset;

-   lockdep_assert_held(>rev_lock);
+   lockdep_assert_held(>rev_lock);

if (a->addr == ADDR_EMPTY || !rblk)
return;

spin_lock(>lock);

-   div_u64_rem(a->addr, rrpc->dev->sec_per_blk, _offset);
+   div_u64_rem(a->addr, rrpc->dev->sec_per_blk, (u32 *)_offset);
WARN_ON(test_and_set_bit(pg_offset, rblk->invalid_pages));
rblk->nr_invalid_pages++;

spin_unlock(>lock);

-   rrpc->rev_trans_map[a->addr - rrpc->poffset].addr = ADDR_EMPTY;
+   pg_offset = lun_poffset(rlun->parent, rrpc->dev);
+   rlun->rev_trans_map[a->addr - pg_offset].addr = ADDR_EMPTY;
  }

  static void rrpc_invalidate_range(struct rrpc *rrpc, sector_t slba,
@@ -52,14 +59,15 @@ static void rrpc_invalidate_range(struct rrpc *rrpc, 
sector_t slba,
  {
sector_t i;

-   spin_lock(>rev_lock);
for (i = slba; i < slba + len; i++) {
struct rrpc_addr *gp = >trans_map[i];
+   struct rrpc_lun *rlun = gp->rblk->rlun;

+   spin_lock(>rev_lock);
rrpc_page_invalidate(rrpc, gp);
+   spin_unlock(>rev_lock);
gp->rblk = NULL;
}
-   spin_unlock(>rev_lock);
  }

  static struct nvm_rq *rrpc_inflight_laddr_acquire(struct rrpc *rrpc,
@@ -116,15 +124,6 @@ static int block_is_full(struct rrpc *rrpc, struct 
rrpc_block *rblk)
return (rblk->next_page == rrpc->dev->sec_per_blk);
  }

-/* Calculate relative addr for the given block, considering instantiated LUNs 
*/
-static u64 block_to_rel_addr(struct rrpc *rrpc, struct rrpc_block *rblk)
-{
-   struct nvm_block *blk = rblk->parent;
-   int lun_blk = blk->id % (rrpc->dev->blks_per_lun * rrpc->nr_luns);
-
-   return lun_blk * rrpc->dev->sec_per_blk;
-}
-
  /* Calculate global addr for the given block */
  static u64 block_to_addr(struct rrpc *rrpc, struct rrpc_block *rblk)
  {
@@ -291,13 +290,14 @@ static void rrpc_end_sync_bio(struct bio *bio)
  static int rrpc_move_valid_pages(struct rrpc *rrpc, struct rrpc_block *rblk)
  {
struct request_queue *q = rrpc->dev->q;
+   struct rrpc_lun *rlun = rblk->rlun;
struct rrpc_rev_addr *rev;
struct nvm_rq *rqd;
struct bio *bio;
struct page *page;
int slot;
int nr_sec_per_blk = rrpc->dev->sec_per_blk;
-   u64 phys_addr;
+   u64 phys_addr, poffset;
DECLARE_COMPLETION_ONSTACK(wait);

if (bitmap_full(rblk->invalid_pages, nr_sec_per_blk))
@@ -315,6 +315,7 @@ static int rrpc_move_valid_pages(struct rrpc *rrpc, struct 
rrpc_block *rblk)
return -ENOMEM;
}

+   poffset = lun_poffset(rlun->parent, rrpc->dev);
while ((slot = find_first_zero_bit(rblk->invalid_pages,
nr_sec_per_blk)) < nr_sec_per_blk) {

@@ -322,23 +323,23 @@ static int rrpc_move_valid_pages(struct rrpc *rrpc, 
struct rrpc_block *rblk)
phys_addr = rblk->parent->id * nr_sec_per_blk + slot;

  try:
-   spin_lock(>rev_lock);
+   spin_lock(>rev_lock);
/* Get logical address from physical to logical table */
-   rev = >rev_trans_map[phys_addr - rrpc->poffset];
+   rev = >rev_trans_map[phys_addr - poffset];
/* already updated by previous regular write */
if (rev->addr == ADDR_EMPTY) {
-

Re: [PATCH v2] lightnvm: divide global reverse translation map into per lun

2016-03-21 Thread Matias Bjørling 

On 03/20/2016 06:51 AM, Wenwei Tao wrote:

Divide the target's global reverse translation map into per lun,
to prepare support for the non-continuous lun target creation.

Signed-off-by: Wenwei Tao 
---

Changes since v1
-fix merge/rebase mistake in rrpc_block_map_update().
-remove variables poffset and lun_offset in rrpc structure
since no one use them now.

  drivers/lightnvm/rrpc.c  | 181 +++
  drivers/lightnvm/rrpc.h  |   9 ++-
  include/linux/lightnvm.h |   1 +
  3 files changed, 111 insertions(+), 80 deletions(-)

diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
index 3ab6495..88e2ce5 100644
--- a/drivers/lightnvm/rrpc.c
+++ b/drivers/lightnvm/rrpc.c
@@ -23,28 +23,35 @@ static int rrpc_submit_io(struct rrpc *rrpc, struct bio 
*bio,
struct nvm_rq *rqd, unsigned long flags);

  #define rrpc_for_each_lun(rrpc, rlun, i) \
-   for ((i) = 0, rlun = &(rrpc)->luns[0]; \
-   (i) < (rrpc)->nr_luns; (i)++, rlun = &(rrpc)->luns[(i)])
+   for ((i) = 0, rlun = &(rrpc)->luns[0]; \
+   (i) < (rrpc)->nr_luns; (i)++, rlun = &(rrpc)->luns[(i)])
+
+static inline u64 lun_poffset(struct nvm_lun *lun, struct nvm_dev *dev)
+{
+   return lun->id * dev->sec_per_lun;
+}

  static void rrpc_page_invalidate(struct rrpc *rrpc, struct rrpc_addr *a)
  {
struct rrpc_block *rblk = a->rblk;
-   unsigned int pg_offset;
+   struct rrpc_lun *rlun = rblk->rlun;
+   u64 pg_offset;

-   lockdep_assert_held(>rev_lock);
+   lockdep_assert_held(>rev_lock);

if (a->addr == ADDR_EMPTY || !rblk)
return;

spin_lock(>lock);

-   div_u64_rem(a->addr, rrpc->dev->sec_per_blk, _offset);
+   div_u64_rem(a->addr, rrpc->dev->sec_per_blk, (u32 *)_offset);
WARN_ON(test_and_set_bit(pg_offset, rblk->invalid_pages));
rblk->nr_invalid_pages++;

spin_unlock(>lock);

-   rrpc->rev_trans_map[a->addr - rrpc->poffset].addr = ADDR_EMPTY;
+   pg_offset = lun_poffset(rlun->parent, rrpc->dev);
+   rlun->rev_trans_map[a->addr - pg_offset].addr = ADDR_EMPTY;
  }

  static void rrpc_invalidate_range(struct rrpc *rrpc, sector_t slba,
@@ -52,14 +59,15 @@ static void rrpc_invalidate_range(struct rrpc *rrpc, 
sector_t slba,
  {
sector_t i;

-   spin_lock(>rev_lock);
for (i = slba; i < slba + len; i++) {
struct rrpc_addr *gp = >trans_map[i];
+   struct rrpc_lun *rlun = gp->rblk->rlun;

+   spin_lock(>rev_lock);
rrpc_page_invalidate(rrpc, gp);
+   spin_unlock(>rev_lock);
gp->rblk = NULL;
}
-   spin_unlock(>rev_lock);
  }

  static struct nvm_rq *rrpc_inflight_laddr_acquire(struct rrpc *rrpc,
@@ -116,15 +124,6 @@ static int block_is_full(struct rrpc *rrpc, struct 
rrpc_block *rblk)
return (rblk->next_page == rrpc->dev->sec_per_blk);
  }

-/* Calculate relative addr for the given block, considering instantiated LUNs 
*/
-static u64 block_to_rel_addr(struct rrpc *rrpc, struct rrpc_block *rblk)
-{
-   struct nvm_block *blk = rblk->parent;
-   int lun_blk = blk->id % (rrpc->dev->blks_per_lun * rrpc->nr_luns);
-
-   return lun_blk * rrpc->dev->sec_per_blk;
-}
-
  /* Calculate global addr for the given block */
  static u64 block_to_addr(struct rrpc *rrpc, struct rrpc_block *rblk)
  {
@@ -291,13 +290,14 @@ static void rrpc_end_sync_bio(struct bio *bio)
  static int rrpc_move_valid_pages(struct rrpc *rrpc, struct rrpc_block *rblk)
  {
struct request_queue *q = rrpc->dev->q;
+   struct rrpc_lun *rlun = rblk->rlun;
struct rrpc_rev_addr *rev;
struct nvm_rq *rqd;
struct bio *bio;
struct page *page;
int slot;
int nr_sec_per_blk = rrpc->dev->sec_per_blk;
-   u64 phys_addr;
+   u64 phys_addr, poffset;
DECLARE_COMPLETION_ONSTACK(wait);

if (bitmap_full(rblk->invalid_pages, nr_sec_per_blk))
@@ -315,6 +315,7 @@ static int rrpc_move_valid_pages(struct rrpc *rrpc, struct 
rrpc_block *rblk)
return -ENOMEM;
}

+   poffset = lun_poffset(rlun->parent, rrpc->dev);
while ((slot = find_first_zero_bit(rblk->invalid_pages,
nr_sec_per_blk)) < nr_sec_per_blk) {

@@ -322,23 +323,23 @@ static int rrpc_move_valid_pages(struct rrpc *rrpc, 
struct rrpc_block *rblk)
phys_addr = rblk->parent->id * nr_sec_per_blk + slot;

  try:
-   spin_lock(>rev_lock);
+   spin_lock(>rev_lock);
/* Get logical address from physical to logical table */
-   rev = >rev_trans_map[phys_addr - rrpc->poffset];
+   rev = >rev_trans_map[phys_addr - poffset];
/* already updated by previous regular write */
if (rev->addr == ADDR_EMPTY) {
-   spin_unlock(>rev_lock);
+