[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]

DQo+IE9uIDYgT2N0IDIwMTgsIGF0IDAzLjEzLCBNYXRpYXMgQmrDuHJsaW5nIDxtYkBsaWdodG52
bS5pbz4gd3JvdGU6DQo+IA0KPj4gT24gMTAvMDQvMjAxOCAwOToxMyBBTSwgSmF2aWVyIEdvbnrD
oWxleiB3cm90ZToNCj4+IENoYW5nZXMgc2luZ2UgVjE6DQo+PiAgIC0gcmVtb3ZlIHNhbml0eSBj
aGVja3Mgb24gdGhlIGZhc3QgcGF0aA0KPj4gVGhpcyBwYXRjaHNldCBpbXBsZW1lbnRzIHN1cHBv
cnQgZm9yIHJldHJpZXZpbmcgY2h1bmsgbWV0YWRhdGEgb24gcmVzZXQuDQo+PiBUaGlzIGlzIHRo
ZSBiYXNlIGZvciBpbXBsZW1lbnRpbmcgd2Vhci1sZXZlbGluZyBhbmQgYWxsb3dpbmcgY2h1bmtz
IHRvDQo+PiBzaHJpbmsgYXQgcnVudGltZS4NCj4+IEphdmllcg0KPj4gSmF2aWVyIEdvbnrDoWxl
eiAoMik6DQo+PiAgIGxpZ2h0bnZtOiBwYmxrOiBhZGQgaGVscGVyIGZvciBwcmludGluZyBjaHVu
ayBzdGF0ZQ0KPj4gICBsaWdodG52bTogcGJsazogcmV0cmlldmUgY2h1bmsgbWV0YWRhdGEgb24g
ZXJhc2UNCj4+ICBkcml2ZXJzL2xpZ2h0bnZtL2NvcmUuYyAgICAgIHwgNDQgKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKysrKy0tDQo+PiAgZHJpdmVycy9saWdodG52bS9wYmxrLWNvcmUu
YyB8IDU0ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tDQo+PiAg
ZHJpdmVycy9saWdodG52bS9wYmxrLmggICAgICB8ICA5ICsrKysrKysrDQo+PiAgMyBmaWxlcyBj
aGFuZ2VkLCA5MiBpbnNlcnRpb25zKCspLCAxNSBkZWxldGlvbnMoLSkNCj4gDQo+IFJpZ2h0IG5v
dyB0aGVyZSBpc24ndCBhIHdheSBpbiB0aGUgc3BlYyB0byB0ZWxsIGlmIHRoZSBkcml2ZSBzdXBw
b3J0cyB0aGlzIGZlYXR1cmUgb3Igbm90LCBhbmQgRFNNIFJlc2V0IGRvZXNuJ3Qgc3VwcG9ydCBp
dCBlaXRoZXIuIE1ha2luZyBpdCBhIGJ1ZyBpbiB0aGUgc3BlYy4gVW50aWwgdGhlIHVwZGF0ZWQg
c3BlYyBpcyBvdXQsIEkgbGlrZSB0byBhdm9pZCBhZGRpbmcgdGhpcyBmZWF0dXJlIGludG8gdGhl
IGtlcm5lbC4NCg0KU3VyZS4gSXQgaXMgdGhlIHNhbWUgYnVnIHdlIGhhdmUgd2hlbiB0aGUgcmVh
ZC93cml0ZSBjb21tYW5kcyBkbyBub3Qgc2lnbmFsIGlmIHRoZSBtZXRhZGF0YSBwb2ludGVyIGlz
IHNldCBvciBub3QuIFRoaXMgaGFzIG5vdCBwcmV2ZW50ZWQgY3VycmVudCBpbXBsZW1lbnRhdGlv
bnMgb2YgdGhpcyBmZWF0dXJlIHRob3VnaC4gDQoNCkFueXdheSwgSSBzZWUgdGhlIHBvaW50OyB0
aGlzIG5vdCBhIGNyaXRpY2FsIHRoaW5nLCBzbyBsZXQgd2FpdCBhbmQgZ2V0IHRoaXMgZml4ZWQg
aW4gdGhlIG5ldyBzcGVjIHJlbGVhc2UuIFRoZXJlIGlzIGEgY291cGxlIG1vcmUgYnVncyB0aGF0
IEkgd291bGQgbGlrZSB0byBmaXguIEnigJlsbCBzZW5kIHlvdSBhbiBlbWFpbCBsYXRlciB0aGlz
IHdlZWsuICANCg0KRm9yIHRoZSBEU00gY29tbWFuZCBzZWVtcyBtb3JlIGRpZmZpY3VsdCBhcyBp
dCB3b3VsZCByZXF1aXJlIGNoYW5nZXMgdG8gdGhlIG52bWUgc3BlYy4gDQoNCj4gDQo+IFRoZSBH
ZXQgTG9nIFBhZ2UgYXBwcm9hY2ggY2FuIGJlIHVzZWQgZm9yIHdlYXItbGV2ZWxsaW5nIGluZm9y
bWF0aW9uLg0KDQpXZWxsLCBpdCBpcyBhbiB1bm5lY2Vzc2FyeSBvdmVyaGVhZCB0byBzZW5kIGEg
bG9nIHBhZ2UgdG8gcHVsbCB0aGUgZGF0YS4gU28gbGV04oCZcyBnZXQgaXQgZml4ZWQuIA0KDQo+
IFNpbWlsYXJseSBmb3Iga25vd2luZyB3aGVuIGEgY2h1bmsgc2hyaW5rLCBwYmxrIGNhbid0IHJl
bHkgb24gYSBjaHVuayBub3Qgc2hyaW5raW5nIGF0IHJ1bnRpbWUgKGFuZCByZXBvcnRpbmcgZWFy
bHkgY2xvc2UgZXJyb3IpLCBzbyBpbiB0aGUgY2FzZSB0aGlzIGhhcHBlbnMsIHBibGsgY2FuIGVp
dGhlciBjb250aW51ZSBidXNpbmVzcyBhcyB1c3VhbCwgb3IgcmVzeW5jIHdpdGggdGhlIGRyaXZl
Lg0KDQpTdXJlLCBpdOKAmXMgZGlmZmVyZW50IGxldmVscyBvZiBoYW5kbGluZyBhbmQgYWxsIHBh
dGhzIHNob3VsZCBiZSBpbXBsZW1lbnRlZC4NCg0KVGhhbmtzLA0KSmF2aWVyDQoNCg==

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