rrpc->nr_sects is calculated after rrpc init luns succeeds,
before that the value of rrpc->nr_sects is zero, so we cannot
use it to calcuate rrpc area size, we use rrpc->nr_luns instead.
Signed-off-by: Wenwei Tao <[email protected]>
---
drivers/lightnvm/rrpc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
index 3ab6495..516a045 100644
--- a/drivers/lightnvm/rrpc.c
+++ b/drivers/lightnvm/rrpc.c
@@ -1223,7 +1223,7 @@ static int rrpc_area_init(struct rrpc *rrpc, sector_t *begin)
{
struct nvm_dev *dev = rrpc->dev;
struct nvmm_type *mt = dev->mt;
- sector_t size = rrpc->nr_sects * dev->sec_size;
+ sector_t size = dev->sec_size * dev->sec_per_lun * rrpc->nr_luns;
size >>= 9;
--
2.7.2.333.g70bd996
Since every target now has their own logical address area,
we should deal trans_map and rev_trans_map with relative
logical address instead of the global one.
Signed-off-by: Wenwei Tao <[email protected]>
---
drivers/lightnvm/rrpc.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
index 516a045..2279bd7 100644
--- a/drivers/lightnvm/rrpc.c
+++ b/drivers/lightnvm/rrpc.c
@@ -998,10 +998,14 @@ static int rrpc_l2p_update(u64 slba, u32 nlb, __le64 *entries, void *private)
{
struct rrpc *rrpc = (struct rrpc *)private;
struct nvm_dev *dev = rrpc->dev;
- struct rrpc_addr *addr = rrpc->trans_map + slba;
- struct rrpc_rev_addr *raddr = rrpc->rev_trans_map;
- u64 elba = slba + nlb;
- u64 i;
+ struct rrpc_addr *addr;
+ struct rrpc_rev_addr *raddr;
+ u64 elba, i;
+
+ slba -= rrpc->soffset >> (ilog2(dev->sec_size) - 9);
+ addr = rrpc->trans_map + slba;
+ raddr = rrpc->rev_trans_map;
+ elba = slba + nlb;
if (unlikely(elba > dev->total_secs)) {
pr_err("nvm: L2P data from device is out of bounds!\n");
--
2.7.2.333.g70bd996
2016-03-30 22:28 GMT+08:00 Wenwei Tao <[email protected]>:
> rrpc->nr_sects is calculated after rrpc init luns succeeds,
> before that the value of rrpc->nr_sects is zero, so we cannot
> use it to calcuate rrpc area size, we use rrpc->nr_luns instead.
>
> Signed-off-by: Wenwei Tao <[email protected]>
> ---
> drivers/lightnvm/rrpc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
> index 3ab6495..516a045 100644
> --- a/drivers/lightnvm/rrpc.c
> +++ b/drivers/lightnvm/rrpc.c
> @@ -1223,7 +1223,7 @@ static int rrpc_area_init(struct rrpc *rrpc, sector_t *begin)
> {
> struct nvm_dev *dev = rrpc->dev;
> struct nvmm_type *mt = dev->mt;
> - sector_t size = rrpc->nr_sects * dev->sec_size;
> + sector_t size = dev->sec_size * dev->sec_per_lun * rrpc->nr_luns;
dev->sec_size * dev->sec_per_lun * rrpc->nr_luns could be oveflow,
should use (sector_t)dev->sec_size * dev->sec_per_lun * rrpc->nr_luns
instead. Will submit another patch to fix it.
>
> size >>= 9;
>
> --
> 2.7.2.333.g70bd996
>
On 03/31/2016 10:31 AM, Wenwei Tao wrote:
> 2016-03-30 22:28 GMT+08:00 Wenwei Tao <[email protected]>:
>> rrpc->nr_sects is calculated after rrpc init luns succeeds,
>> before that the value of rrpc->nr_sects is zero, so we cannot
>> use it to calcuate rrpc area size, we use rrpc->nr_luns instead.
>>
>> Signed-off-by: Wenwei Tao <[email protected]>
>> ---
>> drivers/lightnvm/rrpc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
>> index 3ab6495..516a045 100644
>> --- a/drivers/lightnvm/rrpc.c
>> +++ b/drivers/lightnvm/rrpc.c
>> @@ -1223,7 +1223,7 @@ static int rrpc_area_init(struct rrpc *rrpc, sector_t *begin)
>> {
>> struct nvm_dev *dev = rrpc->dev;
>> struct nvmm_type *mt = dev->mt;
>> - sector_t size = rrpc->nr_sects * dev->sec_size;
>> + sector_t size = dev->sec_size * dev->sec_per_lun * rrpc->nr_luns;
>
> dev->sec_size * dev->sec_per_lun * rrpc->nr_luns could be oveflow,
> should use (sector_t)dev->sec_size * dev->sec_per_lun * rrpc->nr_luns
> instead. Will submit another patch to fix it.
Hi Wenwei,
How about moving rrpc_area_init call under the rrpc_luns_init call
instead. Then nr_sects will have been initialized?
>>
>> size >>= 9;
>>
>> --
>> 2.7.2.333.g70bd996
>>
On 03/30/2016 04:28 PM, Wenwei Tao wrote:
> Since every target now has their own logical address area,
> we should deal trans_map and rev_trans_map with relative
> logical address instead of the global one.
>
> Signed-off-by: Wenwei Tao <[email protected]>
> ---
> drivers/lightnvm/rrpc.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
> index 516a045..2279bd7 100644
> --- a/drivers/lightnvm/rrpc.c
> +++ b/drivers/lightnvm/rrpc.c
> @@ -998,10 +998,14 @@ static int rrpc_l2p_update(u64 slba, u32 nlb, __le64 *entries, void *private)
> {
> struct rrpc *rrpc = (struct rrpc *)private;
> struct nvm_dev *dev = rrpc->dev;
> - struct rrpc_addr *addr = rrpc->trans_map + slba;
> - struct rrpc_rev_addr *raddr = rrpc->rev_trans_map;
> - u64 elba = slba + nlb;
> - u64 i;
> + struct rrpc_addr *addr;
> + struct rrpc_rev_addr *raddr;
> + u64 elba, i;
> +
> + slba -= rrpc->soffset >> (ilog2(dev->sec_size) - 9);
> + addr = rrpc->trans_map + slba;
> + raddr = rrpc->rev_trans_map;
> + elba = slba + nlb;
>
> if (unlikely(elba > dev->total_secs)) {
> pr_err("nvm: L2P data from device is out of bounds!\n");
>
Hi Wenwei,
How about storing the value of "rrpc->soffset >> (ilog2(dev->sec_size) -
9)" in rrpc->soffset. That let us only do the shifting for get/put area
calls.
This could be work, but it needs more steps when rrpc_area_init fails.
If rrpc_area_init fail we may come to rrpc_free and call
rrpc_area_free, we find and put the area by the rrpc->soffset value,
this value is zero when we fail to get an area, we may put an exist
area that really start from zero by mistake. If we move rrpc_area_init
call under the rrpc_luns_init call instead, we need a way to avoid it.
2016-03-31 16:57 GMT+08:00 Matias Bjørling <[email protected]>:
>
>
> On 03/31/2016 10:31 AM, Wenwei Tao wrote:
>>
>> 2016-03-30 22:28 GMT+08:00 Wenwei Tao <[email protected]>:
>>>
>>> rrpc->nr_sects is calculated after rrpc init luns succeeds,
>>> before that the value of rrpc->nr_sects is zero, so we cannot
>>> use it to calcuate rrpc area size, we use rrpc->nr_luns instead.
>>>
>>> Signed-off-by: Wenwei Tao <[email protected]>
>>> ---
>>> drivers/lightnvm/rrpc.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
>>> index 3ab6495..516a045 100644
>>> --- a/drivers/lightnvm/rrpc.c
>>> +++ b/drivers/lightnvm/rrpc.c
>>> @@ -1223,7 +1223,7 @@ static int rrpc_area_init(struct rrpc *rrpc,
>>> sector_t *begin)
>>> {
>>> struct nvm_dev *dev = rrpc->dev;
>>> struct nvmm_type *mt = dev->mt;
>>> - sector_t size = rrpc->nr_sects * dev->sec_size;
>>> + sector_t size = dev->sec_size * dev->sec_per_lun * rrpc->nr_luns;
>>
>>
>> dev->sec_size * dev->sec_per_lun * rrpc->nr_luns could be oveflow,
>> should use (sector_t)dev->sec_size * dev->sec_per_lun * rrpc->nr_luns
>> instead. Will submit another patch to fix it.
>
>
> Hi Wenwei,
>
> How about moving rrpc_area_init call under the rrpc_luns_init call instead.
> Then nr_sects will have been initialized?
>
>
>>>
>>> size >>= 9;
>>>
>>> --
>>> 2.7.2.333.g70bd996
>>>
>
On 03/31/2016 11:51 AM, Wenwei Tao wrote:
> This could be work, but it needs more steps when rrpc_area_init fails.
> If rrpc_area_init fail we may come to rrpc_free and call
> rrpc_area_free, we find and put the area by the rrpc->soffset value,
> this value is zero when we fail to get an area, we may put an exist
> area that really start from zero by mistake. If we move rrpc_area_init
> call under the rrpc_luns_init call instead, we need a way to avoid it.
Fair enough. How about this:
diff --git i/drivers/lightnvm/rrpc.c w/drivers/lightnvm/rrpc.c
index 3ab6495..05a0698 100644
--- i/drivers/lightnvm/rrpc.c
+++ w/drivers/lightnvm/rrpc.c
@@ -1207,10 +1207,6 @@ static int rrpc_luns_init(struct rrpc *rrpc, int
lun_begin, int lun_end)
INIT_WORK(&rlun->ws_gc, rrpc_lun_gc);
spin_lock_init(&rlun->lock);
-
- rrpc->total_blocks += dev->blks_per_lun;
- rrpc->nr_sects += dev->sec_per_lun;
-
}
return 0;
@@ -1388,6 +1384,8 @@ static void *rrpc_init(struct nvm_dev *dev, struct
gendisk *tdisk,
INIT_WORK(&rrpc->ws_requeue, rrpc_requeue);
rrpc->nr_luns = lun_end - lun_begin + 1;
+ rrpc->total_blocks = dev->blks_per_lun * rrpc->nr_luns;
+ rrpc->nr_sects = dev->sec_per_lun * rrpc->nr_luns;
/* simple round-robin strategy */
atomic_set(&rrpc->next_lun, -1);
That nr_sects is initialized and we can use it in rrpc_area_init without
moving the initialization order?
This is okay with me.
By the way, why don't you like (sector_t)dev->sec_size *
dev->sec_per_lun * rrpc->nr_luns, the change seems smaller?
2016-03-31 18:07 GMT+08:00 Matias Bjørling <[email protected]>:
> On 03/31/2016 11:51 AM, Wenwei Tao wrote:
>>
>> This could be work, but it needs more steps when rrpc_area_init fails.
>> If rrpc_area_init fail we may come to rrpc_free and call
>> rrpc_area_free, we find and put the area by the rrpc->soffset value,
>> this value is zero when we fail to get an area, we may put an exist
>> area that really start from zero by mistake. If we move rrpc_area_init
>> call under the rrpc_luns_init call instead, we need a way to avoid it.
>
>
> Fair enough. How about this:
>
> diff --git i/drivers/lightnvm/rrpc.c w/drivers/lightnvm/rrpc.c
> index 3ab6495..05a0698 100644
> --- i/drivers/lightnvm/rrpc.c
> +++ w/drivers/lightnvm/rrpc.c
> @@ -1207,10 +1207,6 @@ static int rrpc_luns_init(struct rrpc *rrpc, int
> lun_begin, int lun_end)
>
> INIT_WORK(&rlun->ws_gc, rrpc_lun_gc);
> spin_lock_init(&rlun->lock);
> -
> - rrpc->total_blocks += dev->blks_per_lun;
> - rrpc->nr_sects += dev->sec_per_lun;
> -
> }
>
> return 0;
> @@ -1388,6 +1384,8 @@ static void *rrpc_init(struct nvm_dev *dev, struct
> gendisk *tdisk,
> INIT_WORK(&rrpc->ws_requeue, rrpc_requeue);
>
> rrpc->nr_luns = lun_end - lun_begin + 1;
> + rrpc->total_blocks = dev->blks_per_lun * rrpc->nr_luns;
> + rrpc->nr_sects = dev->sec_per_lun * rrpc->nr_luns;
>
> /* simple round-robin strategy */
> atomic_set(&rrpc->next_lun, -1);
>
> That nr_sects is initialized and we can use it in rrpc_area_init without
> moving the initialization order?
Okay, I'll send a patch to cover that change.
2016-03-31 17:11 GMT+08:00 Matias Bjørling <[email protected]>:
> On 03/30/2016 04:28 PM, Wenwei Tao wrote:
>>
>> Since every target now has their own logical address area,
>> we should deal trans_map and rev_trans_map with relative
>> logical address instead of the global one.
>>
>> Signed-off-by: Wenwei Tao <[email protected]>
>> ---
>> drivers/lightnvm/rrpc.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/lightnvm/rrpc.c b/drivers/lightnvm/rrpc.c
>> index 516a045..2279bd7 100644
>> --- a/drivers/lightnvm/rrpc.c
>> +++ b/drivers/lightnvm/rrpc.c
>> @@ -998,10 +998,14 @@ static int rrpc_l2p_update(u64 slba, u32 nlb, __le64
>> *entries, void *private)
>> {
>> struct rrpc *rrpc = (struct rrpc *)private;
>> struct nvm_dev *dev = rrpc->dev;
>> - struct rrpc_addr *addr = rrpc->trans_map + slba;
>> - struct rrpc_rev_addr *raddr = rrpc->rev_trans_map;
>> - u64 elba = slba + nlb;
>> - u64 i;
>> + struct rrpc_addr *addr;
>> + struct rrpc_rev_addr *raddr;
>> + u64 elba, i;
>> +
>> + slba -= rrpc->soffset >> (ilog2(dev->sec_size) - 9);
>> + addr = rrpc->trans_map + slba;
>> + raddr = rrpc->rev_trans_map;
>> + elba = slba + nlb;
>>
>> if (unlikely(elba > dev->total_secs)) {
>> pr_err("nvm: L2P data from device is out of bounds!\n");
>>
>
> Hi Wenwei,
>
> How about storing the value of "rrpc->soffset >> (ilog2(dev->sec_size) - 9)"
> in rrpc->soffset. That let us only do the shifting for get/put area calls.
On 03/31/2016 12:26 PM, Wenwei Tao wrote:
> This is okay with me.
> By the way, why don't you like (sector_t)dev->sec_size *
> dev->sec_per_lun * rrpc->nr_luns, the change seems smaller?
Both works. I wanted to fix the underlying issue (which was rrpc values
wasn't initialized enough to be used). Back when luns could be of
different sizes, it made sense. Now that we can have the values up
front, it makes sense to use them. :)