> On 18 Mar 2019, at 13.50, Igor Konopko <igor.j.konopko@xxxxxxxxx> 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@xxxxxxxxx> 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@xxxxxxxxx>
>>>> ---
>>>> 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?
Attachment:
signature.asc
Description: Message signed with OpenPGP
