All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] lightnvm: pblk: add tracing for chunk resets
@ 2018-09-04 10:38 Hans Holmberg
  2018-09-04 10:38 ` [PATCH V3] lightnvm: pblk: fix mapping issue on failed writes Hans Holmberg
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Hans Holmberg @ 2018-09-04 10:38 UTC (permalink / raw)
  To: Matias Bjorling; +Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg

From: Hans Holmberg <hans.holmberg@cnexlabs.com>

Trace state of chunk resets.

Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
Signed-off-by: Matias Bjørling <mb@lightnvm.io>

---

You already picked up the first version of this patch Matias,
but here's a V2 adressing the review comment from Javier that came
after that.

Changes in V2:
	Moved the synchronous PBLK_CHUNK_RESET_START trace to
	pblk_blk_erase_sync (as Javier suggested)

 drivers/lightnvm/pblk-core.c  | 12 ++++++++++++
 drivers/lightnvm/pblk-trace.h | 31 +++++++++++++++++++++++++++++++
 drivers/lightnvm/pblk.h       |  6 ++++++
 3 files changed, 49 insertions(+)

diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 73f117bc99a9..951f81a1b273 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -90,9 +90,15 @@ static void __pblk_end_io_erase(struct pblk *pblk, struct nvm_rq *rqd)
 	atomic_dec(&line->left_seblks);
 
 	if (rqd->error) {
+		trace_pblk_chunk_reset(pblk_disk_name(pblk),
+				&rqd->ppa_addr, PBLK_CHUNK_RESET_FAILED);
+
 		chunk->state = NVM_CHK_ST_OFFLINE;
 		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;
 	}
 
@@ -923,6 +929,9 @@ static int pblk_blk_erase_sync(struct pblk *pblk, struct ppa_addr ppa)
 	struct nvm_rq rqd = {NULL};
 	int ret;
 
+	trace_pblk_chunk_reset(pblk_disk_name(pblk), &ppa,
+				PBLK_CHUNK_RESET_START);
+
 	pblk_setup_e_rq(pblk, &rqd, ppa);
 
 	/* The write thread schedules erases so that it minimizes disturbances
@@ -1736,6 +1745,9 @@ int pblk_blk_erase_async(struct pblk *pblk, struct ppa_addr ppa)
 	rqd->end_io = pblk_end_io_erase;
 	rqd->private = pblk;
 
+	trace_pblk_chunk_reset(pblk_disk_name(pblk),
+				&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.
 	 */
diff --git a/drivers/lightnvm/pblk-trace.h b/drivers/lightnvm/pblk-trace.h
index c171d0450c07..679e5c458ca6 100644
--- a/drivers/lightnvm/pblk-trace.h
+++ b/drivers/lightnvm/pblk-trace.h
@@ -31,6 +31,37 @@ struct ppa_addr;
 	{ PBLK_STATE_RECOVERING,	"RECOVERING",	},	\
 	{ PBLK_STATE_STOPPED,		"STOPPED"	})
 
+#define show_chunk_erase_state(state) __print_symbolic(state,	\
+	{ PBLK_CHUNK_RESET_START,	"START",	},	\
+	{ PBLK_CHUNK_RESET_DONE,	"OK",		},	\
+	{ PBLK_CHUNK_RESET_FAILED,	"FAILED"	})
+
+
+TRACE_EVENT(pblk_chunk_reset,
+
+	TP_PROTO(const char *name, struct ppa_addr *ppa, int state),
+
+	TP_ARGS(name, ppa, state),
+
+	TP_STRUCT__entry(
+		__string(name, name)
+		__field(u64, ppa)
+		__field(int, state);
+	),
+
+	TP_fast_assign(
+		__assign_str(name, name);
+		__entry->ppa = ppa->ppa;
+		__entry->state = state;
+	),
+
+	TP_printk("dev=%s grp=%llu pu=%llu chk=%llu state=%s", __get_str(name),
+			(u64)(((struct ppa_addr *)(&__entry->ppa))->m.grp),
+			(u64)(((struct ppa_addr *)(&__entry->ppa))->m.pu),
+			(u64)(((struct ppa_addr *)(&__entry->ppa))->m.chk),
+			show_chunk_erase_state((int)__entry->state))
+
+);
 
 TRACE_EVENT(pblk_chunk_state,
 
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index ae705fb52cf7..f0fcd963ab3a 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -79,6 +79,12 @@ enum {
 	PBLK_BLK_ST_CLOSED =	0x2,
 };
 
+enum {
+	PBLK_CHUNK_RESET_START,
+	PBLK_CHUNK_RESET_DONE,
+	PBLK_CHUNK_RESET_FAILED,
+};
+
 struct pblk_sec_meta {
 	u64 reserved;
 	__le64 lba;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH V3] lightnvm: pblk: fix mapping issue on failed writes
  2018-09-04 10:38 [PATCH V2] lightnvm: pblk: add tracing for chunk resets Hans Holmberg
@ 2018-09-04 10:38 ` Hans Holmberg
  2018-09-04 10:56   ` Matias Bjørling
  2018-09-04 10:38 ` [PATCH V2] lightnvm: pblk: stop recreating global caches Hans Holmberg
  2018-09-04 10:57 ` [PATCH V2] lightnvm: pblk: add tracing for chunk resets Matias Bjørling
  2 siblings, 1 reply; 6+ messages in thread
From: Hans Holmberg @ 2018-09-04 10:38 UTC (permalink / raw)
  To: Matias Bjorling; +Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg

From: Hans Holmberg <hans.holmberg@cnexlabs.com>

On 1.2-devices, the mapping-out of remaning sectors in the
failed-write's block can result in an infinite loop,
stalling the write pipeline, fix this.

Fixes: 6a3abf5beef6 ("lightnvm: pblk: rework write error recovery path")

Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---

Changes in V2:
	Moved the helper function pblk_next_ppa_in_blk to lightnvm core
	Renamed variable done->last in the helper function.

Changes in V3:
	Renamed the helper function to nvm_next_ppa_in_chk and changed
	the first parameter to type nvm_tgt_dev

 drivers/lightnvm/pblk-write.c | 12 +-----------
 include/linux/lightnvm.h      | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
index 5e6df65d392c..9e8bf2076beb 100644
--- a/drivers/lightnvm/pblk-write.c
+++ b/drivers/lightnvm/pblk-write.c
@@ -106,8 +106,6 @@ static void pblk_complete_write(struct pblk *pblk, struct nvm_rq *rqd,
 /* Map remaining sectors in chunk, starting from ppa */
 static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
 {
-	struct nvm_tgt_dev *dev = pblk->dev;
-	struct nvm_geo *geo = &dev->geo;
 	struct pblk_line *line;
 	struct ppa_addr map_ppa = *ppa;
 	u64 paddr;
@@ -125,15 +123,7 @@ static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
 		if (!test_and_set_bit(paddr, line->invalid_bitmap))
 			le32_add_cpu(line->vsc, -1);
 
-		if (geo->version == NVM_OCSSD_SPEC_12) {
-			map_ppa.ppa++;
-			if (map_ppa.g.pg == geo->num_pg)
-				done = 1;
-		} else {
-			map_ppa.m.sec++;
-			if (map_ppa.m.sec == geo->clba)
-				done = 1;
-		}
+		done = nvm_next_ppa_in_chk(pblk->dev, &map_ppa);
 	}
 
 	line->w_err_gc->has_write_err = 1;
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 09f65c6c6676..36a84180c1e8 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -593,6 +593,42 @@ static inline u32 nvm_ppa64_to_ppa32(struct nvm_dev *dev,
 	return ppa32;
 }
 
+static inline int nvm_next_ppa_in_chk(struct nvm_tgt_dev *dev,
+				      struct ppa_addr *ppa)
+{
+	struct nvm_geo *geo = &dev->geo;
+	int last = 0;
+
+	if (geo->version == NVM_OCSSD_SPEC_12) {
+		int sec = ppa->g.sec;
+
+		sec++;
+		if (sec == geo->ws_min) {
+			int pg = ppa->g.pg;
+
+			sec = 0;
+			pg++;
+			if (pg == geo->num_pg) {
+				int pl = ppa->g.pl;
+
+				pg = 0;
+				pl++;
+				if (pl == geo->num_pln)
+					last = 1;
+
+				ppa->g.pl = pl;
+			}
+			ppa->g.pg = pg;
+		}
+		ppa->g.sec = sec;
+	} else {
+		ppa->m.sec++;
+		if (ppa->m.sec == geo->clba)
+			last = 1;
+	}
+
+	return last;
+}
 
 typedef blk_qc_t (nvm_tgt_make_rq_fn)(struct request_queue *, struct bio *);
 typedef sector_t (nvm_tgt_capacity_fn)(void *);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH V2] lightnvm: pblk: stop recreating global caches
  2018-09-04 10:38 [PATCH V2] lightnvm: pblk: add tracing for chunk resets Hans Holmberg
  2018-09-04 10:38 ` [PATCH V3] lightnvm: pblk: fix mapping issue on failed writes Hans Holmberg
@ 2018-09-04 10:38 ` Hans Holmberg
  2018-09-04 10:54   ` Matias Bjørling
  2018-09-04 10:57 ` [PATCH V2] lightnvm: pblk: add tracing for chunk resets Matias Bjørling
  2 siblings, 1 reply; 6+ messages in thread
From: Hans Holmberg @ 2018-09-04 10:38 UTC (permalink / raw)
  To: Matias Bjorling; +Cc: linux-block, linux-kernel, Javier Gonzales, Hans Holmberg

From: Hans Holmberg <hans.holmberg@cnexlabs.com>

Pblk should not create a set of global caches every time
a pblk instance is created. The global caches should be
made available only when there is one or more pblk instances.

This patch bundles the global caches together with a kref
keeping track of whether the caches should be available or not.

Also, turn the global pblk lock into a mutex that explicitly
protects the caches (as this was the only purpose of the lock).

Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---

Changes in V2:
	* Turned the pblk global lock into a mutex protecting the
	  caches struct.
	* Renamed the global caches to pblk_caches
	* Refactored pblk_get_global_caches to handle only refcounting
 	  and locking.

 drivers/lightnvm/pblk-init.c | 132 ++++++++++++++++++++++++++++---------------
 1 file changed, 86 insertions(+), 46 deletions(-)

diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 9aebdee8e4c9..fb66bc84d5ca 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -26,9 +26,24 @@ static unsigned int write_buffer_size;
 module_param(write_buffer_size, uint, 0644);
 MODULE_PARM_DESC(write_buffer_size, "number of entries in a write buffer");
 
-static struct kmem_cache *pblk_ws_cache, *pblk_rec_cache, *pblk_g_rq_cache,
-				*pblk_w_rq_cache;
-static DECLARE_RWSEM(pblk_lock);
+struct pblk_global_caches {
+	struct kmem_cache	*ws;
+	struct kmem_cache	*rec;
+	struct kmem_cache	*g_rq;
+	struct kmem_cache	*w_rq;
+
+	struct kref		kref;
+
+	struct mutex		mutex; /* Ensures consistency between
+					* caches and kref
+					*/
+};
+
+static struct pblk_global_caches pblk_caches = {
+	.mutex = __MUTEX_INITIALIZER(pblk_caches.mutex),
+	.kref = KREF_INIT(0),
+};
+
 struct bio_set pblk_bio_set;
 
 static int pblk_rw_io(struct request_queue *q, struct pblk *pblk,
@@ -307,53 +322,80 @@ static int pblk_set_addrf(struct pblk *pblk)
 	return 0;
 }
 
-static int pblk_init_global_caches(struct pblk *pblk)
+static int pblk_create_global_caches(void)
 {
-	down_write(&pblk_lock);
-	pblk_ws_cache = kmem_cache_create("pblk_blk_ws",
+
+	pblk_caches.ws = kmem_cache_create("pblk_blk_ws",
 				sizeof(struct pblk_line_ws), 0, 0, NULL);
-	if (!pblk_ws_cache) {
-		up_write(&pblk_lock);
+	if (!pblk_caches.ws)
 		return -ENOMEM;
-	}
 
-	pblk_rec_cache = kmem_cache_create("pblk_rec",
+	pblk_caches.rec = kmem_cache_create("pblk_rec",
 				sizeof(struct pblk_rec_ctx), 0, 0, NULL);
-	if (!pblk_rec_cache) {
-		kmem_cache_destroy(pblk_ws_cache);
-		up_write(&pblk_lock);
-		return -ENOMEM;
-	}
+	if (!pblk_caches.rec)
+		goto fail_destroy_ws;
 
-	pblk_g_rq_cache = kmem_cache_create("pblk_g_rq", pblk_g_rq_size,
+	pblk_caches.g_rq = kmem_cache_create("pblk_g_rq", pblk_g_rq_size,
 				0, 0, NULL);
-	if (!pblk_g_rq_cache) {
-		kmem_cache_destroy(pblk_ws_cache);
-		kmem_cache_destroy(pblk_rec_cache);
-		up_write(&pblk_lock);
-		return -ENOMEM;
-	}
+	if (!pblk_caches.g_rq)
+		goto fail_destroy_rec;
 
-	pblk_w_rq_cache = kmem_cache_create("pblk_w_rq", pblk_w_rq_size,
+	pblk_caches.w_rq = kmem_cache_create("pblk_w_rq", pblk_w_rq_size,
 				0, 0, NULL);
-	if (!pblk_w_rq_cache) {
-		kmem_cache_destroy(pblk_ws_cache);
-		kmem_cache_destroy(pblk_rec_cache);
-		kmem_cache_destroy(pblk_g_rq_cache);
-		up_write(&pblk_lock);
-		return -ENOMEM;
-	}
-	up_write(&pblk_lock);
+	if (!pblk_caches.w_rq)
+		goto fail_destroy_g_rq;
 
 	return 0;
+
+fail_destroy_g_rq:
+	kmem_cache_destroy(pblk_caches.g_rq);
+fail_destroy_rec:
+	kmem_cache_destroy(pblk_caches.rec);
+fail_destroy_ws:
+	kmem_cache_destroy(pblk_caches.ws);
+
+	return -ENOMEM;
+}
+
+static int pblk_get_global_caches(void)
+{
+	int ret;
+
+	mutex_lock(&pblk_caches.mutex);
+
+	if (kref_read(&pblk_caches.kref) > 0) {
+		kref_get(&pblk_caches.kref);
+		mutex_unlock(&pblk_caches.mutex);
+		return 0;
+	}
+
+	ret = pblk_create_global_caches();
+
+	if (!ret)
+		kref_get(&pblk_caches.kref);
+
+	mutex_unlock(&pblk_caches.mutex);
+
+	return ret;
+}
+
+static void pblk_destroy_global_caches(struct kref *ref)
+{
+	struct pblk_global_caches *c;
+
+	c = container_of(ref, struct pblk_global_caches, kref);
+
+	kmem_cache_destroy(c->ws);
+	kmem_cache_destroy(c->rec);
+	kmem_cache_destroy(c->g_rq);
+	kmem_cache_destroy(c->w_rq);
 }
 
-static void pblk_free_global_caches(struct pblk *pblk)
+static void pblk_put_global_caches(void)
 {
-	kmem_cache_destroy(pblk_ws_cache);
-	kmem_cache_destroy(pblk_rec_cache);
-	kmem_cache_destroy(pblk_g_rq_cache);
-	kmem_cache_destroy(pblk_w_rq_cache);
+	mutex_lock(&pblk_caches.mutex);
+	kref_put(&pblk_caches.kref, pblk_destroy_global_caches);
+	mutex_unlock(&pblk_caches.mutex);
 }
 
 static int pblk_core_init(struct pblk *pblk)
@@ -382,7 +424,7 @@ static int pblk_core_init(struct pblk *pblk)
 	if (!pblk->pad_dist)
 		return -ENOMEM;
 
-	if (pblk_init_global_caches(pblk))
+	if (pblk_get_global_caches())
 		goto fail_free_pad_dist;
 
 	/* Internal bios can be at most the sectors signaled by the device. */
@@ -391,27 +433,27 @@ static int pblk_core_init(struct pblk *pblk)
 		goto free_global_caches;
 
 	ret = mempool_init_slab_pool(&pblk->gen_ws_pool, PBLK_GEN_WS_POOL_SIZE,
-				     pblk_ws_cache);
+				     pblk_caches.ws);
 	if (ret)
 		goto free_page_bio_pool;
 
 	ret = mempool_init_slab_pool(&pblk->rec_pool, geo->all_luns,
-				     pblk_rec_cache);
+				     pblk_caches.rec);
 	if (ret)
 		goto free_gen_ws_pool;
 
 	ret = mempool_init_slab_pool(&pblk->r_rq_pool, geo->all_luns,
-				     pblk_g_rq_cache);
+				     pblk_caches.g_rq);
 	if (ret)
 		goto free_rec_pool;
 
 	ret = mempool_init_slab_pool(&pblk->e_rq_pool, geo->all_luns,
-				     pblk_g_rq_cache);
+				     pblk_caches.g_rq);
 	if (ret)
 		goto free_r_rq_pool;
 
 	ret = mempool_init_slab_pool(&pblk->w_rq_pool, geo->all_luns,
-				     pblk_w_rq_cache);
+				     pblk_caches.w_rq);
 	if (ret)
 		goto free_e_rq_pool;
 
@@ -457,7 +499,7 @@ static int pblk_core_init(struct pblk *pblk)
 free_page_bio_pool:
 	mempool_exit(&pblk->page_bio_pool);
 free_global_caches:
-	pblk_free_global_caches(pblk);
+	pblk_put_global_caches();
 fail_free_pad_dist:
 	kfree(pblk->pad_dist);
 	return -ENOMEM;
@@ -481,7 +523,7 @@ static void pblk_core_free(struct pblk *pblk)
 	mempool_exit(&pblk->e_rq_pool);
 	mempool_exit(&pblk->w_rq_pool);
 
-	pblk_free_global_caches(pblk);
+	pblk_put_global_caches();
 	kfree(pblk->pad_dist);
 }
 
@@ -1074,7 +1116,6 @@ static void pblk_exit(void *private, bool graceful)
 {
 	struct pblk *pblk = private;
 
-	down_write(&pblk_lock);
 	pblk_gc_exit(pblk, graceful);
 	pblk_tear_down(pblk, graceful);
 
@@ -1083,7 +1124,6 @@ static void pblk_exit(void *private, bool graceful)
 #endif
 
 	pblk_free(pblk);
-	up_write(&pblk_lock);
 }
 
 static sector_t pblk_capacity(void *private)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] lightnvm: pblk: stop recreating global caches
  2018-09-04 10:38 ` [PATCH V2] lightnvm: pblk: stop recreating global caches Hans Holmberg
@ 2018-09-04 10:54   ` Matias Bjørling
  0 siblings, 0 replies; 6+ messages in thread
From: Matias Bjørling @ 2018-09-04 10:54 UTC (permalink / raw)
  To: hans.ml.holmberg; +Cc: linux-block, linux-kernel, javier, hans.holmberg

On 09/04/2018 12:38 PM, Hans Holmberg wrote:
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
> 
> Pblk should not create a set of global caches every time
> a pblk instance is created. The global caches should be
> made available only when there is one or more pblk instances.
> 
> This patch bundles the global caches together with a kref
> keeping track of whether the caches should be available or not.
> 
> Also, turn the global pblk lock into a mutex that explicitly
> protects the caches (as this was the only purpose of the lock).
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> 
> Changes in V2:
> 	* Turned the pblk global lock into a mutex protecting the
> 	  caches struct.
> 	* Renamed the global caches to pblk_caches
> 	* Refactored pblk_get_global_caches to handle only refcounting
>   	  and locking.
> 
>   drivers/lightnvm/pblk-init.c | 132 ++++++++++++++++++++++++++++---------------
>   1 file changed, 86 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
> index 9aebdee8e4c9..fb66bc84d5ca 100644
> --- a/drivers/lightnvm/pblk-init.c
> +++ b/drivers/lightnvm/pblk-init.c
> @@ -26,9 +26,24 @@ static unsigned int write_buffer_size;
>   module_param(write_buffer_size, uint, 0644);
>   MODULE_PARM_DESC(write_buffer_size, "number of entries in a write buffer");
>   
> -static struct kmem_cache *pblk_ws_cache, *pblk_rec_cache, *pblk_g_rq_cache,
> -				*pblk_w_rq_cache;
> -static DECLARE_RWSEM(pblk_lock);
> +struct pblk_global_caches {
> +	struct kmem_cache	*ws;
> +	struct kmem_cache	*rec;
> +	struct kmem_cache	*g_rq;
> +	struct kmem_cache	*w_rq;
> +
> +	struct kref		kref;
> +
> +	struct mutex		mutex; /* Ensures consistency between
> +					* caches and kref
> +					*/
> +};
> +
> +static struct pblk_global_caches pblk_caches = {
> +	.mutex = __MUTEX_INITIALIZER(pblk_caches.mutex),
> +	.kref = KREF_INIT(0),
> +};
> +
>   struct bio_set pblk_bio_set;
>   
>   static int pblk_rw_io(struct request_queue *q, struct pblk *pblk,
> @@ -307,53 +322,80 @@ static int pblk_set_addrf(struct pblk *pblk)
>   	return 0;
>   }
>   
> -static int pblk_init_global_caches(struct pblk *pblk)
> +static int pblk_create_global_caches(void)
>   {
> -	down_write(&pblk_lock);
> -	pblk_ws_cache = kmem_cache_create("pblk_blk_ws",
> +
> +	pblk_caches.ws = kmem_cache_create("pblk_blk_ws",
>   				sizeof(struct pblk_line_ws), 0, 0, NULL);
> -	if (!pblk_ws_cache) {
> -		up_write(&pblk_lock);
> +	if (!pblk_caches.ws)
>   		return -ENOMEM;
> -	}
>   
> -	pblk_rec_cache = kmem_cache_create("pblk_rec",
> +	pblk_caches.rec = kmem_cache_create("pblk_rec",
>   				sizeof(struct pblk_rec_ctx), 0, 0, NULL);
> -	if (!pblk_rec_cache) {
> -		kmem_cache_destroy(pblk_ws_cache);
> -		up_write(&pblk_lock);
> -		return -ENOMEM;
> -	}
> +	if (!pblk_caches.rec)
> +		goto fail_destroy_ws;
>   
> -	pblk_g_rq_cache = kmem_cache_create("pblk_g_rq", pblk_g_rq_size,
> +	pblk_caches.g_rq = kmem_cache_create("pblk_g_rq", pblk_g_rq_size,
>   				0, 0, NULL);
> -	if (!pblk_g_rq_cache) {
> -		kmem_cache_destroy(pblk_ws_cache);
> -		kmem_cache_destroy(pblk_rec_cache);
> -		up_write(&pblk_lock);
> -		return -ENOMEM;
> -	}
> +	if (!pblk_caches.g_rq)
> +		goto fail_destroy_rec;
>   
> -	pblk_w_rq_cache = kmem_cache_create("pblk_w_rq", pblk_w_rq_size,
> +	pblk_caches.w_rq = kmem_cache_create("pblk_w_rq", pblk_w_rq_size,
>   				0, 0, NULL);
> -	if (!pblk_w_rq_cache) {
> -		kmem_cache_destroy(pblk_ws_cache);
> -		kmem_cache_destroy(pblk_rec_cache);
> -		kmem_cache_destroy(pblk_g_rq_cache);
> -		up_write(&pblk_lock);
> -		return -ENOMEM;
> -	}
> -	up_write(&pblk_lock);
> +	if (!pblk_caches.w_rq)
> +		goto fail_destroy_g_rq;
>   
>   	return 0;
> +
> +fail_destroy_g_rq:
> +	kmem_cache_destroy(pblk_caches.g_rq);
> +fail_destroy_rec:
> +	kmem_cache_destroy(pblk_caches.rec);
> +fail_destroy_ws:
> +	kmem_cache_destroy(pblk_caches.ws);
> +
> +	return -ENOMEM;
> +}
> +
> +static int pblk_get_global_caches(void)
> +{
> +	int ret;
> +
> +	mutex_lock(&pblk_caches.mutex);
> +
> +	if (kref_read(&pblk_caches.kref) > 0) {
> +		kref_get(&pblk_caches.kref);
> +		mutex_unlock(&pblk_caches.mutex);
> +		return 0;
> +	}
> +
> +	ret = pblk_create_global_caches();
> +
> +	if (!ret)
> +		kref_get(&pblk_caches.kref);
> +
> +	mutex_unlock(&pblk_caches.mutex);
> +
> +	return ret;
> +}
> +
> +static void pblk_destroy_global_caches(struct kref *ref)
> +{
> +	struct pblk_global_caches *c;
> +
> +	c = container_of(ref, struct pblk_global_caches, kref);
> +
> +	kmem_cache_destroy(c->ws);
> +	kmem_cache_destroy(c->rec);
> +	kmem_cache_destroy(c->g_rq);
> +	kmem_cache_destroy(c->w_rq);
>   }
>   
> -static void pblk_free_global_caches(struct pblk *pblk)
> +static void pblk_put_global_caches(void)
>   {
> -	kmem_cache_destroy(pblk_ws_cache);
> -	kmem_cache_destroy(pblk_rec_cache);
> -	kmem_cache_destroy(pblk_g_rq_cache);
> -	kmem_cache_destroy(pblk_w_rq_cache);
> +	mutex_lock(&pblk_caches.mutex);
> +	kref_put(&pblk_caches.kref, pblk_destroy_global_caches);
> +	mutex_unlock(&pblk_caches.mutex);
>   }
>   
>   static int pblk_core_init(struct pblk *pblk)
> @@ -382,7 +424,7 @@ static int pblk_core_init(struct pblk *pblk)
>   	if (!pblk->pad_dist)
>   		return -ENOMEM;
>   
> -	if (pblk_init_global_caches(pblk))
> +	if (pblk_get_global_caches())
>   		goto fail_free_pad_dist;
>   
>   	/* Internal bios can be at most the sectors signaled by the device. */
> @@ -391,27 +433,27 @@ static int pblk_core_init(struct pblk *pblk)
>   		goto free_global_caches;
>   
>   	ret = mempool_init_slab_pool(&pblk->gen_ws_pool, PBLK_GEN_WS_POOL_SIZE,
> -				     pblk_ws_cache);
> +				     pblk_caches.ws);
>   	if (ret)
>   		goto free_page_bio_pool;
>   
>   	ret = mempool_init_slab_pool(&pblk->rec_pool, geo->all_luns,
> -				     pblk_rec_cache);
> +				     pblk_caches.rec);
>   	if (ret)
>   		goto free_gen_ws_pool;
>   
>   	ret = mempool_init_slab_pool(&pblk->r_rq_pool, geo->all_luns,
> -				     pblk_g_rq_cache);
> +				     pblk_caches.g_rq);
>   	if (ret)
>   		goto free_rec_pool;
>   
>   	ret = mempool_init_slab_pool(&pblk->e_rq_pool, geo->all_luns,
> -				     pblk_g_rq_cache);
> +				     pblk_caches.g_rq);
>   	if (ret)
>   		goto free_r_rq_pool;
>   
>   	ret = mempool_init_slab_pool(&pblk->w_rq_pool, geo->all_luns,
> -				     pblk_w_rq_cache);
> +				     pblk_caches.w_rq);
>   	if (ret)
>   		goto free_e_rq_pool;
>   
> @@ -457,7 +499,7 @@ static int pblk_core_init(struct pblk *pblk)
>   free_page_bio_pool:
>   	mempool_exit(&pblk->page_bio_pool);
>   free_global_caches:
> -	pblk_free_global_caches(pblk);
> +	pblk_put_global_caches();
>   fail_free_pad_dist:
>   	kfree(pblk->pad_dist);
>   	return -ENOMEM;
> @@ -481,7 +523,7 @@ static void pblk_core_free(struct pblk *pblk)
>   	mempool_exit(&pblk->e_rq_pool);
>   	mempool_exit(&pblk->w_rq_pool);
>   
> -	pblk_free_global_caches(pblk);
> +	pblk_put_global_caches();
>   	kfree(pblk->pad_dist);
>   }
>   
> @@ -1074,7 +1116,6 @@ static void pblk_exit(void *private, bool graceful)
>   {
>   	struct pblk *pblk = private;
>   
> -	down_write(&pblk_lock);
>   	pblk_gc_exit(pblk, graceful);
>   	pblk_tear_down(pblk, graceful);
>   
> @@ -1083,7 +1124,6 @@ static void pblk_exit(void *private, bool graceful)
>   #endif
>   
>   	pblk_free(pblk);
> -	up_write(&pblk_lock);
>   }
>   
>   static sector_t pblk_capacity(void *private)
> 

Thanks. Applied for 4.20.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V3] lightnvm: pblk: fix mapping issue on failed writes
  2018-09-04 10:38 ` [PATCH V3] lightnvm: pblk: fix mapping issue on failed writes Hans Holmberg
@ 2018-09-04 10:56   ` Matias Bjørling
  0 siblings, 0 replies; 6+ messages in thread
From: Matias Bjørling @ 2018-09-04 10:56 UTC (permalink / raw)
  To: hans.ml.holmberg; +Cc: linux-block, linux-kernel, javier, hans.holmberg

On 09/04/2018 12:38 PM, Hans Holmberg wrote:
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
> 
> On 1.2-devices, the mapping-out of remaning sectors in the
> failed-write's block can result in an infinite loop,
> stalling the write pipeline, fix this.
> 
> Fixes: 6a3abf5beef6 ("lightnvm: pblk: rework write error recovery path")
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> ---
> 
> Changes in V2:
> 	Moved the helper function pblk_next_ppa_in_blk to lightnvm core
> 	Renamed variable done->last in the helper function.
> 
> Changes in V3:
> 	Renamed the helper function to nvm_next_ppa_in_chk and changed
> 	the first parameter to type nvm_tgt_dev
> 
>   drivers/lightnvm/pblk-write.c | 12 +-----------
>   include/linux/lightnvm.h      | 36 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
> index 5e6df65d392c..9e8bf2076beb 100644
> --- a/drivers/lightnvm/pblk-write.c
> +++ b/drivers/lightnvm/pblk-write.c
> @@ -106,8 +106,6 @@ static void pblk_complete_write(struct pblk *pblk, struct nvm_rq *rqd,
>   /* Map remaining sectors in chunk, starting from ppa */
>   static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
>   {
> -	struct nvm_tgt_dev *dev = pblk->dev;
> -	struct nvm_geo *geo = &dev->geo;
>   	struct pblk_line *line;
>   	struct ppa_addr map_ppa = *ppa;
>   	u64 paddr;
> @@ -125,15 +123,7 @@ static void pblk_map_remaining(struct pblk *pblk, struct ppa_addr *ppa)
>   		if (!test_and_set_bit(paddr, line->invalid_bitmap))
>   			le32_add_cpu(line->vsc, -1);
>   
> -		if (geo->version == NVM_OCSSD_SPEC_12) {
> -			map_ppa.ppa++;
> -			if (map_ppa.g.pg == geo->num_pg)
> -				done = 1;
> -		} else {
> -			map_ppa.m.sec++;
> -			if (map_ppa.m.sec == geo->clba)
> -				done = 1;
> -		}
> +		done = nvm_next_ppa_in_chk(pblk->dev, &map_ppa);
>   	}
>   
>   	line->w_err_gc->has_write_err = 1;
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index 09f65c6c6676..36a84180c1e8 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -593,6 +593,42 @@ static inline u32 nvm_ppa64_to_ppa32(struct nvm_dev *dev,
>   	return ppa32;
>   }
>   
> +static inline int nvm_next_ppa_in_chk(struct nvm_tgt_dev *dev,
> +				      struct ppa_addr *ppa)
> +{
> +	struct nvm_geo *geo = &dev->geo;
> +	int last = 0;
> +
> +	if (geo->version == NVM_OCSSD_SPEC_12) {
> +		int sec = ppa->g.sec;
> +
> +		sec++;
> +		if (sec == geo->ws_min) {
> +			int pg = ppa->g.pg;
> +
> +			sec = 0;
> +			pg++;
> +			if (pg == geo->num_pg) {
> +				int pl = ppa->g.pl;
> +
> +				pg = 0;
> +				pl++;
> +				if (pl == geo->num_pln)
> +					last = 1;
> +
> +				ppa->g.pl = pl;
> +			}
> +			ppa->g.pg = pg;
> +		}
> +		ppa->g.sec = sec;
> +	} else {
> +		ppa->m.sec++;
> +		if (ppa->m.sec == geo->clba)
> +			last = 1;
> +	}
> +
> +	return last;
> +}
>   
>   typedef blk_qc_t (nvm_tgt_make_rq_fn)(struct request_queue *, struct bio *);
>   typedef sector_t (nvm_tgt_capacity_fn)(void *);
> 

Thanks. Applied for 4.20.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] lightnvm: pblk: add tracing for chunk resets
  2018-09-04 10:38 [PATCH V2] lightnvm: pblk: add tracing for chunk resets Hans Holmberg
  2018-09-04 10:38 ` [PATCH V3] lightnvm: pblk: fix mapping issue on failed writes Hans Holmberg
  2018-09-04 10:38 ` [PATCH V2] lightnvm: pblk: stop recreating global caches Hans Holmberg
@ 2018-09-04 10:57 ` Matias Bjørling
  2 siblings, 0 replies; 6+ messages in thread
From: Matias Bjørling @ 2018-09-04 10:57 UTC (permalink / raw)
  To: hans.ml.holmberg; +Cc: linux-block, linux-kernel, javier, hans.holmberg

On 09/04/2018 12:38 PM, Hans Holmberg wrote:
> From: Hans Holmberg <hans.holmberg@cnexlabs.com>
> 
> Trace state of chunk resets.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> Signed-off-by: Matias Bjørling <mb@lightnvm.io>
> 
> ---
> 
> You already picked up the first version of this patch Matias,
> but here's a V2 adressing the review comment from Javier that came
> after that.
> 
> Changes in V2:
> 	Moved the synchronous PBLK_CHUNK_RESET_START trace to
> 	pblk_blk_erase_sync (as Javier suggested)
> 
>   drivers/lightnvm/pblk-core.c  | 12 ++++++++++++
>   drivers/lightnvm/pblk-trace.h | 31 +++++++++++++++++++++++++++++++
>   drivers/lightnvm/pblk.h       |  6 ++++++
>   3 files changed, 49 insertions(+)
> 
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index 73f117bc99a9..951f81a1b273 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -90,9 +90,15 @@ static void __pblk_end_io_erase(struct pblk *pblk, struct nvm_rq *rqd)
>   	atomic_dec(&line->left_seblks);
>   
>   	if (rqd->error) {
> +		trace_pblk_chunk_reset(pblk_disk_name(pblk),
> +				&rqd->ppa_addr, PBLK_CHUNK_RESET_FAILED);
> +
>   		chunk->state = NVM_CHK_ST_OFFLINE;
>   		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;
>   	}
>   
> @@ -923,6 +929,9 @@ static int pblk_blk_erase_sync(struct pblk *pblk, struct ppa_addr ppa)
>   	struct nvm_rq rqd = {NULL};
>   	int ret;
>   
> +	trace_pblk_chunk_reset(pblk_disk_name(pblk), &ppa,
> +				PBLK_CHUNK_RESET_START);
> +
>   	pblk_setup_e_rq(pblk, &rqd, ppa);
>   
>   	/* The write thread schedules erases so that it minimizes disturbances
> @@ -1736,6 +1745,9 @@ int pblk_blk_erase_async(struct pblk *pblk, struct ppa_addr ppa)
>   	rqd->end_io = pblk_end_io_erase;
>   	rqd->private = pblk;
>   
> +	trace_pblk_chunk_reset(pblk_disk_name(pblk),
> +				&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.
>   	 */
> diff --git a/drivers/lightnvm/pblk-trace.h b/drivers/lightnvm/pblk-trace.h
> index c171d0450c07..679e5c458ca6 100644
> --- a/drivers/lightnvm/pblk-trace.h
> +++ b/drivers/lightnvm/pblk-trace.h
> @@ -31,6 +31,37 @@ struct ppa_addr;
>   	{ PBLK_STATE_RECOVERING,	"RECOVERING",	},	\
>   	{ PBLK_STATE_STOPPED,		"STOPPED"	})
>   
> +#define show_chunk_erase_state(state) __print_symbolic(state,	\
> +	{ PBLK_CHUNK_RESET_START,	"START",	},	\
> +	{ PBLK_CHUNK_RESET_DONE,	"OK",		},	\
> +	{ PBLK_CHUNK_RESET_FAILED,	"FAILED"	})
> +
> +
> +TRACE_EVENT(pblk_chunk_reset,
> +
> +	TP_PROTO(const char *name, struct ppa_addr *ppa, int state),
> +
> +	TP_ARGS(name, ppa, state),
> +
> +	TP_STRUCT__entry(
> +		__string(name, name)
> +		__field(u64, ppa)
> +		__field(int, state);
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(name, name);
> +		__entry->ppa = ppa->ppa;
> +		__entry->state = state;
> +	),
> +
> +	TP_printk("dev=%s grp=%llu pu=%llu chk=%llu state=%s", __get_str(name),
> +			(u64)(((struct ppa_addr *)(&__entry->ppa))->m.grp),
> +			(u64)(((struct ppa_addr *)(&__entry->ppa))->m.pu),
> +			(u64)(((struct ppa_addr *)(&__entry->ppa))->m.chk),
> +			show_chunk_erase_state((int)__entry->state))
> +
> +);
>   
>   TRACE_EVENT(pblk_chunk_state,
>   
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index ae705fb52cf7..f0fcd963ab3a 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -79,6 +79,12 @@ enum {
>   	PBLK_BLK_ST_CLOSED =	0x2,
>   };
>   
> +enum {
> +	PBLK_CHUNK_RESET_START,
> +	PBLK_CHUNK_RESET_DONE,
> +	PBLK_CHUNK_RESET_FAILED,
> +};
> +
>   struct pblk_sec_meta {
>   	u64 reserved;
>   	__le64 lba;
> 

Thanks. Applied for 4.20 (replaced the previous one)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-09-04 15:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-04 10:38 [PATCH V2] lightnvm: pblk: add tracing for chunk resets Hans Holmberg
2018-09-04 10:38 ` [PATCH V3] lightnvm: pblk: fix mapping issue on failed writes Hans Holmberg
2018-09-04 10:56   ` Matias Bjørling
2018-09-04 10:38 ` [PATCH V2] lightnvm: pblk: stop recreating global caches Hans Holmberg
2018-09-04 10:54   ` Matias Bjørling
2018-09-04 10:57 ` [PATCH V2] lightnvm: pblk: add tracing for chunk resets Matias Bjørling

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.