[V2 PATCH 0/2] lightnvm: pblk: retrieve chunk metadata on erase

[V2 PATCH 0/2] lightnvm: pblk: retrieve chunk metadata on erase

[Javier González]

Changes singe V1:
  - remove sanity checks on the fast path

This patchset implements support for retrieving chunk metadata on reset.
This is the base for implementing wear-leveling and allowing chunks to
shrink at runtime.

Javier

Javier González (2):
  lightnvm: pblk: add helper for printing chunk state
  lightnvm: pblk: retrieve chunk metadata on erase

 drivers/lightnvm/core.c      | 44 ++++++++++++++++++++++++++++++++++--
 drivers/lightnvm/pblk-core.c | 54 +++++++++++++++++++++++++++++++++-----------
 drivers/lightnvm/pblk.h      |  9 ++++++++
 3 files changed, 92 insertions(+), 15 deletions(-)

-- 
2.7.4

[PATCH 1/2] lightnvm: pblk: add helper for printing chunk state

[Javier González]

Implement pblk helper for printing chunk state.

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 drivers/lightnvm/pblk.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index 0f98ea24ee59..34c9c1dbeed9 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -1218,6 +1218,15 @@ static inline int pblk_io_aligned(struct pblk *pblk, int nr_secs)
 	return !(nr_secs % pblk->min_write_pgs);
 }
 
+static inline void print_chunk(struct pblk *pblk, struct nvm_chk_meta *chk,
+			       char *msg, int error)
+{
+	pblk_err(pblk, "chunk: (%s: %x) s:%d,t:%d,wi:%d,slba:%llu,cnlb:%llu,wp:%llu\n",
+			msg, error,
+			chk->state, chk->type, chk->wi,
+			chk->slba, chk->cnlb, chk->wp);
+}
+
 #ifdef CONFIG_NVM_PBLK_DEBUG
 static inline void print_ppa(struct pblk *pblk, struct ppa_addr *p,
 			     char *msg, int error)
-- 
2.7.4

[PATCH 2/2] lightnvm: pblk: retrieve chunk metadata on erase

[Javier González]

On the OCSSD 2.0 spec, the device populates the metadata pointer (if
provided) when a chunk is reset. Implement this path in pblk. This is
the base for implementing wear-leveling and supporting variable size
chunks (e.g., due to the device mapping out certain sectors).

For 1.2, reset the write pointer and the state on core so that the erase
path is transparent to pblk wrt OCSSD version.

Signed-off-by: Javier González <javier@cnexlabs.com>
---
 drivers/lightnvm/core.c      | 44 ++++++++++++++++++++++++++++++++++--
 drivers/lightnvm/pblk-core.c | 54 +++++++++++++++++++++++++++++++++-----------
 2 files changed, 83 insertions(+), 15 deletions(-)

diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
index efb976a863d2..dceaae4e795f 100644
--- a/drivers/lightnvm/core.c
+++ b/drivers/lightnvm/core.c
@@ -750,9 +750,40 @@ int nvm_submit_io(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
 }
 EXPORT_SYMBOL(nvm_submit_io);
 
+/* Take only addresses in generic format */
+static void nvm_set_chunk_state_12(struct nvm_dev *dev, struct nvm_rq *rqd)
+{
+	struct ppa_addr *ppa_list = nvm_rq_to_ppa_list(rqd);
+	int i;
+
+	for (i = 0; i < rqd->nr_ppas; i++) {
+		struct ppa_addr ppa;
+		struct nvm_chk_meta *chunk;
+
+		chunk = ((struct nvm_chk_meta *)rqd->meta_list) + i;
+
+		if (rqd->error)
+			chunk->state = NVM_CHK_ST_OFFLINE;
+		else
+			chunk->state = NVM_CHK_ST_FREE;
+
+		chunk->wp = 0;
+		chunk->wi = 0;
+		chunk->type = NVM_CHK_TP_W_SEQ;
+		chunk->cnlb = dev->geo.clba;
+
+		/* recalculate slba for the chunk */
+		ppa = ppa_list[i];
+		ppa.g.pg = ppa.g.pl = ppa.g.sec = 0;
+
+		chunk->slba = generic_to_dev_addr(dev, ppa).ppa;
+	}
+}
+
 int nvm_submit_io_sync(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
 {
 	struct nvm_dev *dev = tgt_dev->parent;
+	struct nvm_geo *geo = &dev->geo;
 	int ret;
 
 	if (!dev->ops->submit_io_sync)
@@ -765,8 +796,12 @@ int nvm_submit_io_sync(struct nvm_tgt_dev *tgt_dev, struct nvm_rq *rqd)
 
 	/* In case of error, fail with right address format */
 	ret = dev->ops->submit_io_sync(dev, rqd);
+
 	nvm_rq_dev_to_tgt(tgt_dev, rqd);
 
+	if (geo->version == NVM_OCSSD_SPEC_12 && rqd->opcode == NVM_OP_ERASE)
+		nvm_set_chunk_state_12(dev, rqd);
+
 	return ret;
 }
 EXPORT_SYMBOL(nvm_submit_io_sync);
@@ -775,10 +810,15 @@ void nvm_end_io(struct nvm_rq *rqd)
 {
 	struct nvm_tgt_dev *tgt_dev = rqd->dev;
 
-	/* Convert address space */
-	if (tgt_dev)
+	if (tgt_dev) {
+		/* Convert address space */
 		nvm_rq_dev_to_tgt(tgt_dev, rqd);
 
+		if (tgt_dev->geo.version == NVM_OCSSD_SPEC_12 &&
+						rqd->opcode == NVM_OP_ERASE)
+			nvm_set_chunk_state_12(tgt_dev->parent, rqd);
+	}
+
 	if (rqd->end_io)
 		rqd->end_io(rqd);
 }
diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 6944aac43b01..2507c4838283 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -80,7 +80,7 @@ static void __pblk_end_io_erase(struct pblk *pblk, struct nvm_rq *rqd)
 {
 	struct nvm_tgt_dev *dev = pblk->dev;
 	struct nvm_geo *geo = &dev->geo;
-	struct nvm_chk_meta *chunk;
+	struct nvm_chk_meta *chunk, *dev_chunk;
 	struct pblk_line *line;
 	int pos;
 
@@ -90,22 +90,42 @@ static void __pblk_end_io_erase(struct pblk *pblk, struct nvm_rq *rqd)
 
 	atomic_dec(&line->left_seblks);
 
+	/* pblk submits a single erase per command */
+	dev_chunk = rqd->meta_list;
+
+	chunk->state = dev_chunk->state;
+	chunk->type = dev_chunk->type;
+	chunk->wi = dev_chunk->wi;
+	chunk->cnlb = dev_chunk->cnlb;
+	chunk->wp = dev_chunk->wp;
+
 	if (rqd->error) {
 		trace_pblk_chunk_reset(pblk_disk_name(pblk),
 				&rqd->ppa_addr, PBLK_CHUNK_RESET_FAILED);
 
-		chunk->state = NVM_CHK_ST_OFFLINE;
+#ifdef CONFIG_NVM_PBLK_DEBUG
+		if (chunk->state != NVM_CHK_ST_OFFLINE)
+			print_chunk(pblk, chunk,
+				"corrupted erase chunk state", rqd->error);
+#endif
+
 		pblk_mark_bb(pblk, line, rqd->ppa_addr);
 	} else {
 		trace_pblk_chunk_reset(pblk_disk_name(pblk),
 				&rqd->ppa_addr, PBLK_CHUNK_RESET_DONE);
 
-		chunk->state = NVM_CHK_ST_FREE;
+#ifdef CONFIG_NVM_PBLK_DEBUG
+		if (chunk->state != NVM_CHK_ST_FREE || chunk->wp ||
+						dev_chunk->slba != chunk->slba)
+			print_chunk(pblk, chunk,
+				"corrupted erase chunk state", rqd->error);
+#endif
 	}
 
 	trace_pblk_chunk_state(pblk_disk_name(pblk), &rqd->ppa_addr,
 				chunk->state);
 
+	pblk_free_rqd_meta(pblk, rqd);
 	atomic_dec(&pblk->inflight_io);
 }
 
@@ -923,14 +943,16 @@ int pblk_line_emeta_read(struct pblk *pblk, struct pblk_line *line,
 	return ret;
 }
 
-static void pblk_setup_e_rq(struct pblk *pblk, struct nvm_rq *rqd,
-			    struct ppa_addr ppa)
+static int pblk_setup_e_rq(struct pblk *pblk, struct nvm_rq *rqd,
+			   struct ppa_addr ppa)
 {
 	rqd->opcode = NVM_OP_ERASE;
 	rqd->ppa_addr = ppa;
 	rqd->nr_ppas = 1;
 	rqd->is_seq = 1;
 	rqd->bio = NULL;
+
+	return pblk_alloc_rqd_meta(pblk, rqd);
 }
 
 static int pblk_blk_erase_sync(struct pblk *pblk, struct ppa_addr ppa)
@@ -938,10 +960,12 @@ static int pblk_blk_erase_sync(struct pblk *pblk, struct ppa_addr ppa)
 	struct nvm_rq rqd = {NULL};
 	int ret;
 
+	ret = pblk_setup_e_rq(pblk, &rqd, ppa);
+	if (ret)
+		return ret;
+
 	trace_pblk_chunk_reset(pblk_disk_name(pblk), &ppa,
-				PBLK_CHUNK_RESET_START);
-
-	pblk_setup_e_rq(pblk, &rqd, ppa);
+						PBLK_CHUNK_RESET_START);
 
 	/* The write thread schedules erases so that it minimizes disturbances
 	 * with writes. Thus, there is no need to take the LUN semaphore.
@@ -1746,11 +1770,15 @@ void pblk_line_put_wq(struct kref *ref)
 int pblk_blk_erase_async(struct pblk *pblk, struct ppa_addr ppa)
 {
 	struct nvm_rq *rqd;
-	int err;
+	int ret;
 
 	rqd = pblk_alloc_rqd(pblk, PBLK_ERASE);
 
-	pblk_setup_e_rq(pblk, rqd, ppa);
+	ret = pblk_setup_e_rq(pblk, rqd, ppa);
+	if (ret) {
+		pblk_free_rqd(pblk, rqd, PBLK_ERASE);
+		return ret;
+	}
 
 	rqd->end_io = pblk_end_io_erase;
 	rqd->private = pblk;
@@ -1761,8 +1789,8 @@ int pblk_blk_erase_async(struct pblk *pblk, struct ppa_addr ppa)
 	/* The write thread schedules erases so that it minimizes disturbances
 	 * with writes. Thus, there is no need to take the LUN semaphore.
 	 */
-	err = pblk_submit_io(pblk, rqd);
-	if (err) {
+	ret = pblk_submit_io(pblk, rqd);
+	if (ret) {
 		struct nvm_tgt_dev *dev = pblk->dev;
 		struct nvm_geo *geo = &dev->geo;
 
@@ -1771,7 +1799,7 @@ int pblk_blk_erase_async(struct pblk *pblk, struct ppa_addr ppa)
 					pblk_ppa_to_pos(geo, ppa));
 	}
 
-	return err;
+	return ret;
 }
 
 struct pblk_line *pblk_line_get_data(struct pblk *pblk)
-- 
2.7.4

Re: [V2 PATCH 0/2] lightnvm: pblk: retrieve chunk metadata on erase

[Matias Bjørling]

On 10/04/2018 09:13 AM, Javier González wrote:
> Changes singe V1:
>    - remove sanity checks on the fast path
> 
> This patchset implements support for retrieving chunk metadata on reset.
> This is the base for implementing wear-leveling and allowing chunks to
> shrink at runtime.
> 
> Javier
> 
> Javier González (2):
>    lightnvm: pblk: add helper for printing chunk state
>    lightnvm: pblk: retrieve chunk metadata on erase
> 
>   drivers/lightnvm/core.c      | 44 ++++++++++++++++++++++++++++++++++--
>   drivers/lightnvm/pblk-core.c | 54 +++++++++++++++++++++++++++++++++-----------
>   drivers/lightnvm/pblk.h      |  9 ++++++++
>   3 files changed, 92 insertions(+), 15 deletions(-)
> 

Right now there isn't a way in the spec to tell if the drive supports 
this feature or not, and DSM Reset doesn't support it either. Making it 
a bug in the spec. Until the updated spec is out, I like to avoid adding 
this feature into the kernel.

The Get Log Page approach can be used for wear-levelling information. 
Similarly for knowing when a chunk shrink, pblk can't rely on a chunk 
not shrinking at runtime (and reporting early close error), so in the 
case this happens, pblk can either continue business as usual, or resync 
with the drive.

Re: [V2 PATCH 0/2] lightnvm: pblk: retrieve chunk metadata on erase

[Javier Gonzalez]

> On 6 Oct 2018, at 03.13, Matias Bjørling <mb@lightnvm.io> wrote:
> 
>> On 10/04/2018 09:13 AM, Javier González wrote:
>> Changes singe V1:
>>   - remove sanity checks on the fast path
>> This patchset implements support for retrieving chunk metadata on reset.
>> This is the base for implementing wear-leveling and allowing chunks to
>> shrink at runtime.
>> Javier
>> Javier González (2):
>>   lightnvm: pblk: add helper for printing chunk state
>>   lightnvm: pblk: retrieve chunk metadata on erase
>>  drivers/lightnvm/core.c      | 44 ++++++++++++++++++++++++++++++++++--
>>  drivers/lightnvm/pblk-core.c | 54 +++++++++++++++++++++++++++++++++-----------
>>  drivers/lightnvm/pblk.h      |  9 ++++++++
>>  3 files changed, 92 insertions(+), 15 deletions(-)
> 
> Right now there isn't a way in the spec to tell if the drive supports this feature or not, and DSM Reset doesn't support it either. Making it a bug in the spec. Until the updated spec is out, I like to avoid adding this feature into the kernel.

Sure. It is the same bug we have when the read/write commands do not signal if the metadata pointer is set or not. This has not prevented current implementations of this feature though. 

Anyway, I see the point; this not a critical thing, so let wait and get this fixed in the new spec release. There is a couple more bugs that I would like to fix. I’ll send you an email later this week.  

For the DSM command seems more difficult as it would require changes to the nvme spec. 

> 
> The Get Log Page approach can be used for wear-levelling information.

Well, it is an unnecessary overhead to send a log page to pull the data. So let’s get it fixed. 

> Similarly for knowing when a chunk shrink, pblk can't rely on a chunk not shrinking at runtime (and reporting early close error), so in the case this happens, pblk can either continue business as usual, or resync with the drive.

Sure, it’s different levels of handling and all paths should be implemented.

Thanks,
Javier