> On 18 Mar 2019, at 13.50, Igor Konopko <igor.j.konopko@intel.com> wrote:
> 
> 
> 
> On 17.03.2019 20:24, Matias Bjørling wrote:
>> 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.
> 
> So based on OCSSD 2.0 spec "If WP = SLBA + NLB, the chunk is closed" (also on "Figure 5: Chunk State Diagram in spec": WP = SLBA + NLB for closed chunk), my understanding it that the WP is not valid for that particular use case, since in written_secs we want to obtain NLB field (relative within chunk, without starting LBA value). So that's why I used fixed CLBA here, since it WP pointer for closed chunk is absolute value instead of relative one.
> Did I misunderstood sth in the spec?

I see where the confusion comes from. I have always understood it in the
way that WP points to the next available sector in the chunk. When WP =
SLBA + NLB, then the WP remains in the last valid sector (as there are
no more available sectors).

Both polk and QEMU are implemented this way, but it might be worth a
clarification in the spec (to be one way or the other)? Matias?