Re: [PATCH v2 2/2] lightnvm: add non-continuous lun target creation support

Matias Bjørling
2016-01-28T09:09:50Z

On 01/28/2016 09:50 AM, Wenwei Tao wrote:
> 2016-01-27 17:44 GMT+08:00 Matias Bjørling <mb@lightnvm.io>:
>> On 01/26/2016 01:33 PM, Wenwei Tao wrote:
>>> When create a target, we specify the begin lunid and
>>> the end lunid, and get the corresponding continuous
>>> luns from media manager, if one of the luns is not free,
>>> we failed to create the target, even if the device's
>>> total free luns are enough.
>>>
>>> So add non-continuous lun target creation support,
>>> thus we can improve the backend device's space utilization.
>>> Signed-off-by: Wenwei Tao <ww.tao0320@gmail.com>
>>> ---
>>> Changes since v1:
>>> -use NVM_FIXED instead NVM_C_FIXED in gennvm_get_lun
>>> -add target creation flags check
>>> -rebase to v4.5-rc1
>>>
>>> drivers/lightnvm/core.c | 36 ++++---
>>> drivers/lightnvm/gennvm.c | 42 ++++++++-
>>> drivers/lightnvm/rrpc.c | 215 +++++++++++++++++++++++++++---------------
>>> drivers/lightnvm/rrpc.h | 6 +-
>>> include/linux/lightnvm.h | 24 ++++-
>>> include/uapi/linux/lightnvm.h | 3 +
>>> 6 files changed, 229 insertions(+), 97 deletions(-)
>>>
>>
>> Hi Wenwei,
>>
>> I did some digging on the patch and changed the interface to a
>> reserve/release interface. I also removed the logic to dynamically
>> select another lun than the one requested.
>>
>> A couple of questions:
>>
>> 1. The rrpc_lun->rev_lock and rev_trans_map change; this might be for
>> another patch, and it isn't directly related to continuous mapping?
>
> rrpc_lun->rev_lock and rev_trans_map change is related to
> non-continuous mapping, it's not directly related to continuous
> mapping.
> Put this change in another patch along with non-continuous mapping
> support and this patch would be only add reserve/release thing, is
> that your suggestion?

Yes, that would be great. Then we keep it separate. I'll like to do some
benchmarks with the patch on and off, to see the performance difference.

>
>> 2. Instead of dynamically assigning new luns when not available, what
>> about taking a list of lun ids instead?
>>
>
> Seems you prefer user make the choice ?

Yes, I want it to be deterministic. For example, if we do it
dynamically, the user might first allocate 2-4, and then allocate 1-3 ,
which will actually allocate 0,1,5. Then later, a user tries to allocate
on 0, and instead gets returned 6. It quickly makes it difficult to use.

> But the target creation can still fail if one of the list lun ids is
> not available although there may be enough free luns.

Agree, the user would have to look up the free luns and then resubmit
the target allocation.