| Message ID | 20190227171442.11853-7-igor.j.konopko@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lightnvm: bugfixes and improvements | expand |
> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> wrote: > > In current pblk implementation of erase command > there is a chance tha sector bits are set to some > random values for erase PPA. This is unexpected > situation, since erase shall be always chunk > aligned. This patch fixes that issue > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > --- > drivers/lightnvm/pblk-core.c | 1 + > drivers/lightnvm/pblk-map.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > index a98b2255f963..78b1eea4ab67 100644 > --- a/drivers/lightnvm/pblk-core.c > +++ b/drivers/lightnvm/pblk-core.c > @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct pblk_line *line) > > ppa = pblk->luns[bit].bppa; /* set ch and lun */ > ppa.a.blk = line->id; > + ppa.a.reserved = 0; > > atomic_dec(&line->left_eblks); > WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); > diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c > index 79df583ea709..aea46b4ec40f 100644 > --- a/drivers/lightnvm/pblk-map.c > +++ b/drivers/lightnvm/pblk-map.c > @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, > > *erase_ppa = ppa_list[i]; > erase_ppa->a.blk = e_line->id; > + erase_ppa->a.reserved = 0; > > spin_unlock(&e_line->lock); > > @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, > atomic_dec(&e_line->left_eblks); > *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ > erase_ppa->a.blk = e_line->id; > + erase_ppa->a.reserved = 0; > } > > return 0; > -- > 2.17.1 I’m fine with adding this, but note that there is actually no requirement for the erase to be chunk aligned - the only bits that should be looked at are group, PU and chunk. Reviewed-by: Javier González <javier@javigon.com>
I strongly disagree with adding code that would mask implantation errors. If we want more internal checks, we could add an if statement that would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> wrote: > > > On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> wrote: > > > > In current pblk implementation of erase command > > there is a chance tha sector bits are set to some > > random values for erase PPA. This is unexpected > > situation, since erase shall be always chunk > > aligned. This patch fixes that issue > > > > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > > --- > > drivers/lightnvm/pblk-core.c | 1 + > > drivers/lightnvm/pblk-map.c | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > > index a98b2255f963..78b1eea4ab67 100644 > > --- a/drivers/lightnvm/pblk-core.c > > +++ b/drivers/lightnvm/pblk-core.c > > @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct pblk_line *line) > > > > ppa = pblk->luns[bit].bppa; /* set ch and lun */ > > ppa.a.blk = line->id; > > + ppa.a.reserved = 0; > > > > atomic_dec(&line->left_eblks); > > WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); > > diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c > > index 79df583ea709..aea46b4ec40f 100644 > > --- a/drivers/lightnvm/pblk-map.c > > +++ b/drivers/lightnvm/pblk-map.c > > @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, > > > > *erase_ppa = ppa_list[i]; > > erase_ppa->a.blk = e_line->id; > > + erase_ppa->a.reserved = 0; > > > > spin_unlock(&e_line->lock); > > > > @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, > > atomic_dec(&e_line->left_eblks); > > *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ > > erase_ppa->a.blk = e_line->id; > > + erase_ppa->a.reserved = 0; > > } > > > > return 0; > > -- > > 2.17.1 > > I’m fine with adding this, but note that there is actually no > requirement for the erase to be chunk aligned - the only bits that > should be looked at are group, PU and chunk. > > Reviewed-by: Javier González <javier@javigon.com> > >
> On 4 Mar 2019, at 10.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote: > > I strongly disagree with adding code that would mask implantation errors. > > If we want more internal checks, we could add an if statement that > would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. > Not sure who this is for - better not to top post. In any case, this is a spec grey zone. I’m ok with cleaning the bits as they mean nothing for the reset command. If you feel that strongly about this, you can take if with Igor. > > On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> wrote: >>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> wrote: >>> >>> In current pblk implementation of erase command >>> there is a chance tha sector bits are set to some >>> random values for erase PPA. This is unexpected >>> situation, since erase shall be always chunk >>> aligned. This patch fixes that issue >>> >>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> >>> --- >>> drivers/lightnvm/pblk-core.c | 1 + >>> drivers/lightnvm/pblk-map.c | 2 ++ >>> 2 files changed, 3 insertions(+) >>> >>> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c >>> index a98b2255f963..78b1eea4ab67 100644 >>> --- a/drivers/lightnvm/pblk-core.c >>> +++ b/drivers/lightnvm/pblk-core.c >>> @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct pblk_line *line) >>> >>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ >>> ppa.a.blk = line->id; >>> + ppa.a.reserved = 0; >>> >>> atomic_dec(&line->left_eblks); >>> WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); >>> diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c >>> index 79df583ea709..aea46b4ec40f 100644 >>> --- a/drivers/lightnvm/pblk-map.c >>> +++ b/drivers/lightnvm/pblk-map.c >>> @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, >>> >>> *erase_ppa = ppa_list[i]; >>> erase_ppa->a.blk = e_line->id; >>> + erase_ppa->a.reserved = 0; >>> >>> spin_unlock(&e_line->lock); >>> >>> @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, >>> atomic_dec(&e_line->left_eblks); >>> *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ >>> erase_ppa->a.blk = e_line->id; >>> + erase_ppa->a.reserved = 0; >>> } >>> >>> return 0; >>> -- >>> 2.17.1 >> >> I’m fine with adding this, but note that there is actually no >> requirement for the erase to be chunk aligned - the only bits that >> should be looked at are group, PU and chunk. >> >> Reviewed-by: Javier González <javier@javigon.com>
On Mon, Mar 4, 2019 at 10:11 AM Javier González <javier@javigon.com> wrote: > > > On 4 Mar 2019, at 10.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote: > > > > I strongly disagree with adding code that would mask implantation errors. > > > > If we want more internal checks, we could add an if statement that > > would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. > > > > Not sure who this is for - better not to top post. > > In any case, this is a spec grey zone. I’m ok with cleaning the bits as > they mean nothing for the reset command. If you feel that strongly about > this, you can take if with Igor. Pardon the top-post. It was meant for both you and Igor. > > > > > On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> wrote: > >>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> wrote: > >>> > >>> In current pblk implementation of erase command > >>> there is a chance tha sector bits are set to some > >>> random values for erase PPA. This is unexpected > >>> situation, since erase shall be always chunk > >>> aligned. This patch fixes that issue > >>> > >>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > >>> --- > >>> drivers/lightnvm/pblk-core.c | 1 + > >>> drivers/lightnvm/pblk-map.c | 2 ++ > >>> 2 files changed, 3 insertions(+) > >>> > >>> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > >>> index a98b2255f963..78b1eea4ab67 100644 > >>> --- a/drivers/lightnvm/pblk-core.c > >>> +++ b/drivers/lightnvm/pblk-core.c > >>> @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct pblk_line *line) > >>> > >>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>> ppa.a.blk = line->id; > >>> + ppa.a.reserved = 0; > >>> > >>> atomic_dec(&line->left_eblks); > >>> WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); > >>> diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c > >>> index 79df583ea709..aea46b4ec40f 100644 > >>> --- a/drivers/lightnvm/pblk-map.c > >>> +++ b/drivers/lightnvm/pblk-map.c > >>> @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, > >>> > >>> *erase_ppa = ppa_list[i]; > >>> erase_ppa->a.blk = e_line->id; > >>> + erase_ppa->a.reserved = 0; > >>> > >>> spin_unlock(&e_line->lock); > >>> > >>> @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, > >>> atomic_dec(&e_line->left_eblks); > >>> *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>> erase_ppa->a.blk = e_line->id; > >>> + erase_ppa->a.reserved = 0; > >>> } > >>> > >>> return 0; > >>> -- > >>> 2.17.1 > >> > >> I’m fine with adding this, but note that there is actually no > >> requirement for the erase to be chunk aligned - the only bits that > >> should be looked at are group, PU and chunk. > >> > >> Reviewed-by: Javier González <javier@javigon.com>
On 04.03.2019 12:43, Hans Holmberg wrote: > On Mon, Mar 4, 2019 at 10:11 AM Javier González <javier@javigon.com> wrote: >> >>> On 4 Mar 2019, at 10.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote: >>> >>> I strongly disagree with adding code that would mask implantation errors. >>> >>> If we want more internal checks, we could add an if statement that >>> would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. >>> >> >> Not sure who this is for - better not to top post. >> >> In any case, this is a spec grey zone. I’m ok with cleaning the bits as >> they mean nothing for the reset command. If you feel that strongly about >> this, you can take if with Igor. > > Pardon the top-post. It was meant for both you and Igor. > OCSSD 2.0 spec for vector chunk reset (chapter 2.2.2) explicitly says "The addresses in the LBA list shall be the first logical block address of each chunk to be reset.". So in my understanding we suppose to clear the sectors bits of the PPA address in order to be spec compliant. >> >>> >>> On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> wrote: >>>>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> wrote: >>>>> >>>>> In current pblk implementation of erase command >>>>> there is a chance tha sector bits are set to some >>>>> random values for erase PPA. This is unexpected >>>>> situation, since erase shall be always chunk >>>>> aligned. This patch fixes that issue >>>>> >>>>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> >>>>> --- >>>>> drivers/lightnvm/pblk-core.c | 1 + >>>>> drivers/lightnvm/pblk-map.c | 2 ++ >>>>> 2 files changed, 3 insertions(+) >>>>> >>>>> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c >>>>> index a98b2255f963..78b1eea4ab67 100644 >>>>> --- a/drivers/lightnvm/pblk-core.c >>>>> +++ b/drivers/lightnvm/pblk-core.c >>>>> @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct pblk_line *line) >>>>> >>>>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ >>>>> ppa.a.blk = line->id; >>>>> + ppa.a.reserved = 0; >>>>> >>>>> atomic_dec(&line->left_eblks); >>>>> WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); >>>>> diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c >>>>> index 79df583ea709..aea46b4ec40f 100644 >>>>> --- a/drivers/lightnvm/pblk-map.c >>>>> +++ b/drivers/lightnvm/pblk-map.c >>>>> @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, >>>>> >>>>> *erase_ppa = ppa_list[i]; >>>>> erase_ppa->a.blk = e_line->id; >>>>> + erase_ppa->a.reserved = 0; >>>>> >>>>> spin_unlock(&e_line->lock); >>>>> >>>>> @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, >>>>> atomic_dec(&e_line->left_eblks); >>>>> *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ >>>>> erase_ppa->a.blk = e_line->id; >>>>> + erase_ppa->a.reserved = 0; >>>>> } >>>>> >>>>> return 0; >>>>> -- >>>>> 2.17.1 >>>> >>>> I’m fine with adding this, but note that there is actually no >>>> requirement for the erase to be chunk aligned - the only bits that >>>> should be looked at are group, PU and chunk. >>>> >>>> Reviewed-by: Javier González <javier@javigon.com>
On Mon, Mar 4, 2019 at 1:44 PM Igor Konopko <igor.j.konopko@intel.com> wrote: > > > > On 04.03.2019 12:43, Hans Holmberg wrote: > > On Mon, Mar 4, 2019 at 10:11 AM Javier González <javier@javigon.com> wrote: > >> > >>> On 4 Mar 2019, at 10.05, Hans Holmberg <hans.ml.holmberg@owltronix.com> wrote: > >>> > >>> I strongly disagree with adding code that would mask implantation errors. > >>> > >>> If we want more internal checks, we could add an if statement that > >>> would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. > >>> > >> > >> Not sure who this is for - better not to top post. > >> > >> In any case, this is a spec grey zone. I’m ok with cleaning the bits as > >> they mean nothing for the reset command. If you feel that strongly about > >> this, you can take if with Igor. > > > > Pardon the top-post. It was meant for both you and Igor. > > > > OCSSD 2.0 spec for vector chunk reset (chapter 2.2.2) explicitly says > "The addresses in the LBA list shall be the first logical block address > of each chunk to be reset.". So in my understanding we suppose to clear > the sectors bits of the PPA address in order to be spec compliant. Hmm.. but my point is that if pblk->luns[bit].bppa is set to a valid chunk address, there is no need to clear a.reserved. that address originates from core.c:196, and thats valid. > > >> > >>> > >>> On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> wrote: > >>>>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> wrote: > >>>>> > >>>>> In current pblk implementation of erase command > >>>>> there is a chance tha sector bits are set to some > >>>>> random values for erase PPA. This is unexpected > >>>>> situation, since erase shall be always chunk > >>>>> aligned. This patch fixes that issue > >>>>> > >>>>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > >>>>> --- > >>>>> drivers/lightnvm/pblk-core.c | 1 + > >>>>> drivers/lightnvm/pblk-map.c | 2 ++ > >>>>> 2 files changed, 3 insertions(+) > >>>>> > >>>>> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c > >>>>> index a98b2255f963..78b1eea4ab67 100644 > >>>>> --- a/drivers/lightnvm/pblk-core.c > >>>>> +++ b/drivers/lightnvm/pblk-core.c > >>>>> @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct pblk_line *line) > >>>>> > >>>>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>>>> ppa.a.blk = line->id; > >>>>> + ppa.a.reserved = 0; > >>>>> > >>>>> atomic_dec(&line->left_eblks); > >>>>> WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); > >>>>> diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c > >>>>> index 79df583ea709..aea46b4ec40f 100644 > >>>>> --- a/drivers/lightnvm/pblk-map.c > >>>>> +++ b/drivers/lightnvm/pblk-map.c > >>>>> @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, > >>>>> > >>>>> *erase_ppa = ppa_list[i]; > >>>>> erase_ppa->a.blk = e_line->id; > >>>>> + erase_ppa->a.reserved = 0; > >>>>> > >>>>> spin_unlock(&e_line->lock); > >>>>> > >>>>> @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, > >>>>> atomic_dec(&e_line->left_eblks); > >>>>> *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>>>> erase_ppa->a.blk = e_line->id; > >>>>> + erase_ppa->a.reserved = 0; > >>>>> } > >>>>> > >>>>> return 0; > >>>>> -- > >>>>> 2.17.1 > >>>> > >>>> I’m fine with adding this, but note that there is actually no > >>>> requirement for the erase to be chunk aligned - the only bits that > >>>> should be looked at are group, PU and chunk. > >>>> > >>>> Reviewed-by: Javier González <javier@javigon.com>
On 3/4/19 1:44 PM, Igor Konopko wrote: > > > On 04.03.2019 12:43, Hans Holmberg wrote: >> On Mon, Mar 4, 2019 at 10:11 AM Javier González <javier@javigon.com> >> wrote: >>> >>>> On 4 Mar 2019, at 10.05, Hans Holmberg >>>> <hans.ml.holmberg@owltronix.com> wrote: >>>> >>>> I strongly disagree with adding code that would mask implantation >>>> errors. >>>> >>>> If we want more internal checks, we could add an if statement that >>>> would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. >>>> >>> >>> Not sure who this is for - better not to top post. >>> >>> In any case, this is a spec grey zone. I’m ok with cleaning the bits as >>> they mean nothing for the reset command. If you feel that strongly about >>> this, you can take if with Igor. >> >> Pardon the top-post. It was meant for both you and Igor. >> > > OCSSD 2.0 spec for vector chunk reset (chapter 2.2.2) explicitly says > "The addresses in the LBA list shall be the first logical block address > of each chunk to be reset.". So in my understanding we suppose to clear > the sectors bits of the PPA address in order to be spec compliant. > Agree. And since ppa_addr is allocated on the stack, it should be either memset or the remaining fields should be set to 0. Maybe better to zero initialize in declaration? >>> >>>> >>>> On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> >>>> wrote: >>>>>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> >>>>>> wrote: >>>>>> >>>>>> In current pblk implementation of erase command >>>>>> there is a chance tha sector bits are set to some >>>>>> random values for erase PPA. This is unexpected >>>>>> situation, since erase shall be always chunk >>>>>> aligned. This patch fixes that issue >>>>>> >>>>>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> >>>>>> --- >>>>>> drivers/lightnvm/pblk-core.c | 1 + >>>>>> drivers/lightnvm/pblk-map.c | 2 ++ >>>>>> 2 files changed, 3 insertions(+) >>>>>> >>>>>> diff --git a/drivers/lightnvm/pblk-core.c >>>>>> b/drivers/lightnvm/pblk-core.c >>>>>> index a98b2255f963..78b1eea4ab67 100644 >>>>>> --- a/drivers/lightnvm/pblk-core.c >>>>>> +++ b/drivers/lightnvm/pblk-core.c >>>>>> @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct >>>>>> pblk_line *line) >>>>>> >>>>>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ >>>>>> ppa.a.blk = line->id; >>>>>> + ppa.a.reserved = 0; >>>>>> >>>>>> atomic_dec(&line->left_eblks); >>>>>> WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); >>>>>> diff --git a/drivers/lightnvm/pblk-map.c >>>>>> b/drivers/lightnvm/pblk-map.c >>>>>> index 79df583ea709..aea46b4ec40f 100644 >>>>>> --- a/drivers/lightnvm/pblk-map.c >>>>>> +++ b/drivers/lightnvm/pblk-map.c >>>>>> @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, >>>>>> struct nvm_rq *rqd, >>>>>> >>>>>> *erase_ppa = ppa_list[i]; >>>>>> erase_ppa->a.blk = e_line->id; >>>>>> + erase_ppa->a.reserved = 0; >>>>>> >>>>>> spin_unlock(&e_line->lock); >>>>>> >>>>>> @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, >>>>>> struct nvm_rq *rqd, >>>>>> atomic_dec(&e_line->left_eblks); >>>>>> *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ >>>>>> erase_ppa->a.blk = e_line->id; >>>>>> + erase_ppa->a.reserved = 0; >>>>>> } >>>>>> >>>>>> return 0; >>>>>> -- >>>>>> 2.17.1 >>>>> >>>>> I’m fine with adding this, but note that there is actually no >>>>> requirement for the erase to be chunk aligned - the only bits that >>>>> should be looked at are group, PU and chunk. >>>>> >>>>> Reviewed-by: Javier González <javier@javigon.com>
On Mon, Mar 4, 2019 at 2:00 PM Matias Bjørling <mb@lightnvm.io> wrote: > > On 3/4/19 1:44 PM, Igor Konopko wrote: > > > > > > On 04.03.2019 12:43, Hans Holmberg wrote: > >> On Mon, Mar 4, 2019 at 10:11 AM Javier González <javier@javigon.com> > >> wrote: > >>> > >>>> On 4 Mar 2019, at 10.05, Hans Holmberg > >>>> <hans.ml.holmberg@owltronix.com> wrote: > >>>> > >>>> I strongly disagree with adding code that would mask implantation > >>>> errors. > >>>> > >>>> If we want more internal checks, we could add an if statement that > >>>> would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. > >>>> > >>> > >>> Not sure who this is for - better not to top post. > >>> > >>> In any case, this is a spec grey zone. I’m ok with cleaning the bits as > >>> they mean nothing for the reset command. If you feel that strongly about > >>> this, you can take if with Igor. > >> > >> Pardon the top-post. It was meant for both you and Igor. > >> > > > > OCSSD 2.0 spec for vector chunk reset (chapter 2.2.2) explicitly says > > "The addresses in the LBA list shall be the first logical block address > > of each chunk to be reset.". So in my understanding we suppose to clear > > the sectors bits of the PPA address in order to be spec compliant. > > > > Agree. And since ppa_addr is allocated on the stack, it should be either > memset or the remaining fields should be set to 0. Maybe better to zero > initialize in declaration? Ah, I thought this was not needed, as ppa is initialized as: ppa = pblk->luns[bit].bppa; /* set ch and lun */ and luns[bit].bppa is initialized to on a value that originally comes from drivers/lightnvm/core.c:196 (and that's explicitly zeroing all 64 bits before setting ch and lun) Let me know if i don't make sense here. > > >>> > >>>> > >>>> On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> > >>>> wrote: > >>>>>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> > >>>>>> wrote: > >>>>>> > >>>>>> In current pblk implementation of erase command > >>>>>> there is a chance tha sector bits are set to some > >>>>>> random values for erase PPA. This is unexpected > >>>>>> situation, since erase shall be always chunk > >>>>>> aligned. This patch fixes that issue > >>>>>> > >>>>>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > >>>>>> --- > >>>>>> drivers/lightnvm/pblk-core.c | 1 + > >>>>>> drivers/lightnvm/pblk-map.c | 2 ++ > >>>>>> 2 files changed, 3 insertions(+) > >>>>>> > >>>>>> diff --git a/drivers/lightnvm/pblk-core.c > >>>>>> b/drivers/lightnvm/pblk-core.c > >>>>>> index a98b2255f963..78b1eea4ab67 100644 > >>>>>> --- a/drivers/lightnvm/pblk-core.c > >>>>>> +++ b/drivers/lightnvm/pblk-core.c > >>>>>> @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct > >>>>>> pblk_line *line) > >>>>>> > >>>>>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>>>>> ppa.a.blk = line->id; > >>>>>> + ppa.a.reserved = 0; > >>>>>> > >>>>>> atomic_dec(&line->left_eblks); > >>>>>> WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); > >>>>>> diff --git a/drivers/lightnvm/pblk-map.c > >>>>>> b/drivers/lightnvm/pblk-map.c > >>>>>> index 79df583ea709..aea46b4ec40f 100644 > >>>>>> --- a/drivers/lightnvm/pblk-map.c > >>>>>> +++ b/drivers/lightnvm/pblk-map.c > >>>>>> @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, > >>>>>> struct nvm_rq *rqd, > >>>>>> > >>>>>> *erase_ppa = ppa_list[i]; > >>>>>> erase_ppa->a.blk = e_line->id; > >>>>>> + erase_ppa->a.reserved = 0; > >>>>>> > >>>>>> spin_unlock(&e_line->lock); > >>>>>> > >>>>>> @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, > >>>>>> struct nvm_rq *rqd, > >>>>>> atomic_dec(&e_line->left_eblks); > >>>>>> *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>>>>> erase_ppa->a.blk = e_line->id; > >>>>>> + erase_ppa->a.reserved = 0; > >>>>>> } > >>>>>> > >>>>>> return 0; > >>>>>> -- > >>>>>> 2.17.1 > >>>>> > >>>>> I’m fine with adding this, but note that there is actually no > >>>>> requirement for the erase to be chunk aligned - the only bits that > >>>>> should be looked at are group, PU and chunk. > >>>>> > >>>>> Reviewed-by: Javier González <javier@javigon.com> >
On 05.03.2019 09:20, Hans Holmberg wrote: > On Mon, Mar 4, 2019 at 2:00 PM Matias Bjørling <mb@lightnvm.io> wrote: >> >> On 3/4/19 1:44 PM, Igor Konopko wrote: >>> >>> >>> On 04.03.2019 12:43, Hans Holmberg wrote: >>>> On Mon, Mar 4, 2019 at 10:11 AM Javier González <javier@javigon.com> >>>> wrote: >>>>> >>>>>> On 4 Mar 2019, at 10.05, Hans Holmberg >>>>>> <hans.ml.holmberg@owltronix.com> wrote: >>>>>> >>>>>> I strongly disagree with adding code that would mask implantation >>>>>> errors. >>>>>> >>>>>> If we want more internal checks, we could add an if statement that >>>>>> would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. >>>>>> >>>>> >>>>> Not sure who this is for - better not to top post. >>>>> >>>>> In any case, this is a spec grey zone. I’m ok with cleaning the bits as >>>>> they mean nothing for the reset command. If you feel that strongly about >>>>> this, you can take if with Igor. >>>> >>>> Pardon the top-post. It was meant for both you and Igor. >>>> >>> >>> OCSSD 2.0 spec for vector chunk reset (chapter 2.2.2) explicitly says >>> "The addresses in the LBA list shall be the first logical block address >>> of each chunk to be reset.". So in my understanding we suppose to clear >>> the sectors bits of the PPA address in order to be spec compliant. >>> >> >> Agree. And since ppa_addr is allocated on the stack, it should be either >> memset or the remaining fields should be set to 0. Maybe better to zero >> initialize in declaration? > > Ah, I thought this was not needed, as ppa is initialized as: > > ppa = pblk->luns[bit].bppa; /* set ch and lun */ > > and luns[bit].bppa is initialized to on a value that originally comes > from drivers/lightnvm/core.c:196 > (and that's explicitly zeroing all 64 bits before setting ch and lun) > > Let me know if i don't make sense here. > I just noticed the same. In two places (pblk-core:1095 and pblk-map:205) we are using values initialized previously in core.c - so my changes are not needed here. But still there is one place (pblk-map:163) where we initializing erase_ppa based on ppa_list[i], which has PPA sector set in most of the cases, so this zeroing is still needed here. >> >>>>> >>>>>> >>>>>> On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> >>>>>> wrote: >>>>>>>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> >>>>>>>> wrote: >>>>>>>> >>>>>>>> In current pblk implementation of erase command >>>>>>>> there is a chance tha sector bits are set to some >>>>>>>> random values for erase PPA. This is unexpected >>>>>>>> situation, since erase shall be always chunk >>>>>>>> aligned. This patch fixes that issue >>>>>>>> >>>>>>>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> >>>>>>>> --- >>>>>>>> drivers/lightnvm/pblk-core.c | 1 + >>>>>>>> drivers/lightnvm/pblk-map.c | 2 ++ >>>>>>>> 2 files changed, 3 insertions(+) >>>>>>>> >>>>>>>> diff --git a/drivers/lightnvm/pblk-core.c >>>>>>>> b/drivers/lightnvm/pblk-core.c >>>>>>>> index a98b2255f963..78b1eea4ab67 100644 >>>>>>>> --- a/drivers/lightnvm/pblk-core.c >>>>>>>> +++ b/drivers/lightnvm/pblk-core.c >>>>>>>> @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct >>>>>>>> pblk_line *line) >>>>>>>> >>>>>>>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ >>>>>>>> ppa.a.blk = line->id; >>>>>>>> + ppa.a.reserved = 0; >>>>>>>> >>>>>>>> atomic_dec(&line->left_eblks); >>>>>>>> WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); >>>>>>>> diff --git a/drivers/lightnvm/pblk-map.c >>>>>>>> b/drivers/lightnvm/pblk-map.c >>>>>>>> index 79df583ea709..aea46b4ec40f 100644 >>>>>>>> --- a/drivers/lightnvm/pblk-map.c >>>>>>>> +++ b/drivers/lightnvm/pblk-map.c >>>>>>>> @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, >>>>>>>> struct nvm_rq *rqd, >>>>>>>> >>>>>>>> *erase_ppa = ppa_list[i]; >>>>>>>> erase_ppa->a.blk = e_line->id; >>>>>>>> + erase_ppa->a.reserved = 0; >>>>>>>> >>>>>>>> spin_unlock(&e_line->lock); >>>>>>>> >>>>>>>> @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, >>>>>>>> struct nvm_rq *rqd, >>>>>>>> atomic_dec(&e_line->left_eblks); >>>>>>>> *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ >>>>>>>> erase_ppa->a.blk = e_line->id; >>>>>>>> + erase_ppa->a.reserved = 0; >>>>>>>> } >>>>>>>> >>>>>>>> return 0; >>>>>>>> -- >>>>>>>> 2.17.1 >>>>>>> >>>>>>> I’m fine with adding this, but note that there is actually no >>>>>>> requirement for the erase to be chunk aligned - the only bits that >>>>>>> should be looked at are group, PU and chunk. >>>>>>> >>>>>>> Reviewed-by: Javier González <javier@javigon.com> >>
On Tue, Mar 5, 2019 at 9:26 AM Igor Konopko <igor.j.konopko@intel.com> wrote: > > > > On 05.03.2019 09:20, Hans Holmberg wrote: > > On Mon, Mar 4, 2019 at 2:00 PM Matias Bjørling <mb@lightnvm.io> wrote: > >> > >> On 3/4/19 1:44 PM, Igor Konopko wrote: > >>> > >>> > >>> On 04.03.2019 12:43, Hans Holmberg wrote: > >>>> On Mon, Mar 4, 2019 at 10:11 AM Javier González <javier@javigon.com> > >>>> wrote: > >>>>> > >>>>>> On 4 Mar 2019, at 10.05, Hans Holmberg > >>>>>> <hans.ml.holmberg@owltronix.com> wrote: > >>>>>> > >>>>>> I strongly disagree with adding code that would mask implantation > >>>>>> errors. > >>>>>> > >>>>>> If we want more internal checks, we could add an if statement that > >>>>>> would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. > >>>>>> > >>>>> > >>>>> Not sure who this is for - better not to top post. > >>>>> > >>>>> In any case, this is a spec grey zone. I’m ok with cleaning the bits as > >>>>> they mean nothing for the reset command. If you feel that strongly about > >>>>> this, you can take if with Igor. > >>>> > >>>> Pardon the top-post. It was meant for both you and Igor. > >>>> > >>> > >>> OCSSD 2.0 spec for vector chunk reset (chapter 2.2.2) explicitly says > >>> "The addresses in the LBA list shall be the first logical block address > >>> of each chunk to be reset.". So in my understanding we suppose to clear > >>> the sectors bits of the PPA address in order to be spec compliant. > >>> > >> > >> Agree. And since ppa_addr is allocated on the stack, it should be either > >> memset or the remaining fields should be set to 0. Maybe better to zero > >> initialize in declaration? > > > > Ah, I thought this was not needed, as ppa is initialized as: > > > > ppa = pblk->luns[bit].bppa; /* set ch and lun */ > > > > and luns[bit].bppa is initialized to on a value that originally comes > > from drivers/lightnvm/core.c:196 > > (and that's explicitly zeroing all 64 bits before setting ch and lun) > > > > Let me know if i don't make sense here. > > > > I just noticed the same. > > In two places (pblk-core:1095 and pblk-map:205) we are using values > initialized previously in core.c - so my changes are not needed here. > > But still there is one place (pblk-map:163) where we initializing > erase_ppa based on ppa_list[i], which has PPA sector set in most of the > cases, so this zeroing is still needed here. Yes, you are right, thanks for pointing it out. Are you ok with just changing this? > >> > >>>>> > >>>>>> > >>>>>> On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> > >>>>>> wrote: > >>>>>>>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> > >>>>>>>> wrote: > >>>>>>>> > >>>>>>>> In current pblk implementation of erase command > >>>>>>>> there is a chance tha sector bits are set to some > >>>>>>>> random values for erase PPA. This is unexpected > >>>>>>>> situation, since erase shall be always chunk > >>>>>>>> aligned. This patch fixes that issue > >>>>>>>> > >>>>>>>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > >>>>>>>> --- > >>>>>>>> drivers/lightnvm/pblk-core.c | 1 + > >>>>>>>> drivers/lightnvm/pblk-map.c | 2 ++ > >>>>>>>> 2 files changed, 3 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/drivers/lightnvm/pblk-core.c > >>>>>>>> b/drivers/lightnvm/pblk-core.c > >>>>>>>> index a98b2255f963..78b1eea4ab67 100644 > >>>>>>>> --- a/drivers/lightnvm/pblk-core.c > >>>>>>>> +++ b/drivers/lightnvm/pblk-core.c > >>>>>>>> @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct > >>>>>>>> pblk_line *line) > >>>>>>>> > >>>>>>>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>>>>>>> ppa.a.blk = line->id; > >>>>>>>> + ppa.a.reserved = 0; > >>>>>>>> > >>>>>>>> atomic_dec(&line->left_eblks); > >>>>>>>> WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); > >>>>>>>> diff --git a/drivers/lightnvm/pblk-map.c > >>>>>>>> b/drivers/lightnvm/pblk-map.c > >>>>>>>> index 79df583ea709..aea46b4ec40f 100644 > >>>>>>>> --- a/drivers/lightnvm/pblk-map.c > >>>>>>>> +++ b/drivers/lightnvm/pblk-map.c > >>>>>>>> @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, > >>>>>>>> struct nvm_rq *rqd, > >>>>>>>> > >>>>>>>> *erase_ppa = ppa_list[i]; > >>>>>>>> erase_ppa->a.blk = e_line->id; > >>>>>>>> + erase_ppa->a.reserved = 0; > >>>>>>>> > >>>>>>>> spin_unlock(&e_line->lock); > >>>>>>>> > >>>>>>>> @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, > >>>>>>>> struct nvm_rq *rqd, > >>>>>>>> atomic_dec(&e_line->left_eblks); > >>>>>>>> *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>>>>>>> erase_ppa->a.blk = e_line->id; > >>>>>>>> + erase_ppa->a.reserved = 0; > >>>>>>>> } > >>>>>>>> > >>>>>>>> return 0; > >>>>>>>> -- > >>>>>>>> 2.17.1 > >>>>>>> > >>>>>>> I’m fine with adding this, but note that there is actually no > >>>>>>> requirement for the erase to be chunk aligned - the only bits that > >>>>>>> should be looked at are group, PU and chunk. > >>>>>>> > >>>>>>> Reviewed-by: Javier González <javier@javigon.com> > >>
On Tue, Mar 5, 2019 at 9:51 AM Igor Konopko <igor.j.konopko@intel.com> wrote: > > > > On 05.03.2019 09:40, Hans Holmberg wrote: > > On Tue, Mar 5, 2019 at 9:26 AM Igor Konopko <igor.j.konopko@intel.com> wrote: > >> > >> > >> > >> On 05.03.2019 09:20, Hans Holmberg wrote: > >>> On Mon, Mar 4, 2019 at 2:00 PM Matias Bjørling <mb@lightnvm.io> wrote: > >>>> > >>>> On 3/4/19 1:44 PM, Igor Konopko wrote: > >>>>> > >>>>> > >>>>> On 04.03.2019 12:43, Hans Holmberg wrote: > >>>>>> On Mon, Mar 4, 2019 at 10:11 AM Javier González <javier@javigon.com> > >>>>>> wrote: > >>>>>>> > >>>>>>>> On 4 Mar 2019, at 10.05, Hans Holmberg > >>>>>>>> <hans.ml.holmberg@owltronix.com> wrote: > >>>>>>>> > >>>>>>>> I strongly disagree with adding code that would mask implantation > >>>>>>>> errors. > >>>>>>>> > >>>>>>>> If we want more internal checks, we could add an if statement that > >>>>>>>> would only be compiled in if CONFIG_NVM_PBLK_DEBUG is enabled. > >>>>>>>> > >>>>>>> > >>>>>>> Not sure who this is for - better not to top post. > >>>>>>> > >>>>>>> In any case, this is a spec grey zone. I’m ok with cleaning the bits as > >>>>>>> they mean nothing for the reset command. If you feel that strongly about > >>>>>>> this, you can take if with Igor. > >>>>>> > >>>>>> Pardon the top-post. It was meant for both you and Igor. > >>>>>> > >>>>> > >>>>> OCSSD 2.0 spec for vector chunk reset (chapter 2.2.2) explicitly says > >>>>> "The addresses in the LBA list shall be the first logical block address > >>>>> of each chunk to be reset.". So in my understanding we suppose to clear > >>>>> the sectors bits of the PPA address in order to be spec compliant. > >>>>> > >>>> > >>>> Agree. And since ppa_addr is allocated on the stack, it should be either > >>>> memset or the remaining fields should be set to 0. Maybe better to zero > >>>> initialize in declaration? > >>> > >>> Ah, I thought this was not needed, as ppa is initialized as: > >>> > >>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>> > >>> and luns[bit].bppa is initialized to on a value that originally comes > >>> from drivers/lightnvm/core.c:196 > >>> (and that's explicitly zeroing all 64 bits before setting ch and lun) > >>> > >>> Let me know if i don't make sense here. > >>> > >> > >> I just noticed the same. > >> > >> In two places (pblk-core:1095 and pblk-map:205) we are using values > >> initialized previously in core.c - so my changes are not needed here. > >> > >> But still there is one place (pblk-map:163) where we initializing > >> erase_ppa based on ppa_list[i], which has PPA sector set in most of the > >> cases, so this zeroing is still needed here. > > > > Yes, you are right, thanks for pointing it out. Are you ok with just > > changing this? > > Yes, that's my plan for v2. Great, thanks. > > > >>>> > >>>>>>> > >>>>>>>> > >>>>>>>> On Mon, Mar 4, 2019 at 8:48 AM Javier González <javier@javigon.com> > >>>>>>>> wrote: > >>>>>>>>>> On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@intel.com> > >>>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>> In current pblk implementation of erase command > >>>>>>>>>> there is a chance tha sector bits are set to some > >>>>>>>>>> random values for erase PPA. This is unexpected > >>>>>>>>>> situation, since erase shall be always chunk > >>>>>>>>>> aligned. This patch fixes that issue > >>>>>>>>>> > >>>>>>>>>> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > >>>>>>>>>> --- > >>>>>>>>>> drivers/lightnvm/pblk-core.c | 1 + > >>>>>>>>>> drivers/lightnvm/pblk-map.c | 2 ++ > >>>>>>>>>> 2 files changed, 3 insertions(+) > >>>>>>>>>> > >>>>>>>>>> diff --git a/drivers/lightnvm/pblk-core.c > >>>>>>>>>> b/drivers/lightnvm/pblk-core.c > >>>>>>>>>> index a98b2255f963..78b1eea4ab67 100644 > >>>>>>>>>> --- a/drivers/lightnvm/pblk-core.c > >>>>>>>>>> +++ b/drivers/lightnvm/pblk-core.c > >>>>>>>>>> @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct > >>>>>>>>>> pblk_line *line) > >>>>>>>>>> > >>>>>>>>>> ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>>>>>>>>> ppa.a.blk = line->id; > >>>>>>>>>> + ppa.a.reserved = 0; > >>>>>>>>>> > >>>>>>>>>> atomic_dec(&line->left_eblks); > >>>>>>>>>> WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); > >>>>>>>>>> diff --git a/drivers/lightnvm/pblk-map.c > >>>>>>>>>> b/drivers/lightnvm/pblk-map.c > >>>>>>>>>> index 79df583ea709..aea46b4ec40f 100644 > >>>>>>>>>> --- a/drivers/lightnvm/pblk-map.c > >>>>>>>>>> +++ b/drivers/lightnvm/pblk-map.c > >>>>>>>>>> @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, > >>>>>>>>>> struct nvm_rq *rqd, > >>>>>>>>>> > >>>>>>>>>> *erase_ppa = ppa_list[i]; > >>>>>>>>>> erase_ppa->a.blk = e_line->id; > >>>>>>>>>> + erase_ppa->a.reserved = 0; > >>>>>>>>>> > >>>>>>>>>> spin_unlock(&e_line->lock); > >>>>>>>>>> > >>>>>>>>>> @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, > >>>>>>>>>> struct nvm_rq *rqd, > >>>>>>>>>> atomic_dec(&e_line->left_eblks); > >>>>>>>>>> *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ > >>>>>>>>>> erase_ppa->a.blk = e_line->id; > >>>>>>>>>> + erase_ppa->a.reserved = 0; > >>>>>>>>>> } > >>>>>>>>>> > >>>>>>>>>> return 0; > >>>>>>>>>> -- > >>>>>>>>>> 2.17.1 > >>>>>>>>> > >>>>>>>>> I’m fine with adding this, but note that there is actually no > >>>>>>>>> requirement for the erase to be chunk aligned - the only bits that > >>>>>>>>> should be looked at are group, PU and chunk. > >>>>>>>>> > >>>>>>>>> Reviewed-by: Javier González <javier@javigon.com> > >>>>
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c index a98b2255f963..78b1eea4ab67 100644 --- a/drivers/lightnvm/pblk-core.c +++ b/drivers/lightnvm/pblk-core.c @@ -978,6 +978,7 @@ int pblk_line_erase(struct pblk *pblk, struct pblk_line *line) ppa = pblk->luns[bit].bppa; /* set ch and lun */ ppa.a.blk = line->id; + ppa.a.reserved = 0; atomic_dec(&line->left_eblks); WARN_ON(test_and_set_bit(bit, line->erase_bitmap)); diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c index 79df583ea709..aea46b4ec40f 100644 --- a/drivers/lightnvm/pblk-map.c +++ b/drivers/lightnvm/pblk-map.c @@ -161,6 +161,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, *erase_ppa = ppa_list[i]; erase_ppa->a.blk = e_line->id; + erase_ppa->a.reserved = 0; spin_unlock(&e_line->lock); @@ -202,6 +203,7 @@ int pblk_map_erase_rq(struct pblk *pblk, struct nvm_rq *rqd, atomic_dec(&e_line->left_eblks); *erase_ppa = pblk->luns[bit].bppa; /* set ch and lun */ erase_ppa->a.blk = e_line->id; + erase_ppa->a.reserved = 0; } return 0;
In current pblk implementation of erase command there is a chance tha sector bits are set to some random values for erase PPA. This is unexpected situation, since erase shall be always chunk aligned. This patch fixes that issue Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> --- drivers/lightnvm/pblk-core.c | 1 + drivers/lightnvm/pblk-map.c | 2 ++ 2 files changed, 3 insertions(+)