Re: [PATCH 04/18] lightnvm: pblk: OOB recovery for closed chunks fix

["Matias Bjørling" <mb@lightnvm.io>]

On 3/16/19 3:43 PM, Javier González wrote:
>> On 14 Mar 2019, at 09.04, Igor Konopko <igor.j.konopko@intel.com> wrote:
>>
>> In case of OOB recovery, when some of the chunks are in closed state,
>> we are calculating number of written sectors in line incorrectly,
>> because we are always counting chunk WP, which for closed chunks
>> does not longer reflects written sectors in particular chunks. This
>> patch for such a chunks takes clba field instead.
>>
>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com>
>> ---
>> drivers/lightnvm/pblk-recovery.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
>> index 83b467b..bcd3633 100644
>> --- a/drivers/lightnvm/pblk-recovery.c
>> +++ b/drivers/lightnvm/pblk-recovery.c
>> @@ -101,6 +101,8 @@ static void pblk_update_line_wp(struct pblk *pblk, struct pblk_line *line,
>>
>> static u64 pblk_sec_in_open_line(struct pblk *pblk, struct pblk_line *line)
>> {
>> +	struct nvm_tgt_dev *dev = pblk->dev;
>> +	struct nvm_geo *geo = &dev->geo;
>> 	struct pblk_line_meta *lm = &pblk->lm;
>> 	int nr_bb = bitmap_weight(line->blk_bitmap, lm->blk_per_line);
>> 	u64 written_secs = 0;
>> @@ -113,7 +115,11 @@ static u64 pblk_sec_in_open_line(struct pblk *pblk, struct pblk_line *line)
>> 		if (chunk->state & NVM_CHK_ST_OFFLINE)
>> 			continue;
>>
>> -		written_secs += chunk->wp;
>> +		if (chunk->state & NVM_CHK_ST_OPEN)
>> +			written_secs += chunk->wp;
>> +		else if (chunk->state & NVM_CHK_ST_CLOSED)
>> +			written_secs += geo->clba;
>> +
>> 		valid_chunks++;
>> 	}
>>
>> --
>> 2.9.5
> 
> Mmmm. The change is correct, but can you develop on why the WP does not
> reflect the written sectors in a closed chunk? As I understand it, the
> WP reflects the last written sector; if it happens to be WP == clba, then
> the chunk state machine transitions to closed, but the WP remains
> untouched. It is only when we reset the chunk that the WP comes back to
> 0. Am I missing something?
> 

I agree with Javier. In OCSSD, the write pointer shall always be valid.