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@xxxxxxxxxxx>
wrote:
On 4 Mar 2019, at 10.05, Hans Holmberg
<hans.ml.holmberg@xxxxxxxxxxxxx> 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@xxxxxxxxxxx>
wrote:
On 27 Feb 2019, at 18.14, Igor Konopko <igor.j.konopko@xxxxxxxxx>
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@xxxxxxxxx>
---
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@xxxxxxxxxxx>
