linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 0/8] lightnvm updates for 5.1
@ 2019-02-11 12:25 Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 1/8] lightnvm: pblk: stop taking the free lock in in pblk_lines_free Matias Bjørling
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Matias Bjørling @ 2019-02-11 12:25 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-kernel, Matias Bjørling

Hi Jens,

Would you please pick up the following patches for 5.1?

It is a bunch of misc patches this time. A couple of fixes and cleanups.

Andy Shevchenko (2):
  lightnvm: Use u64 instead of __le64 for CPU visible side
  lightnvm: pblk: Switch to use new generic UUID API

Hans Holmberg (3):
  lightnvm: pblk: stop taking the free lock in in pblk_lines_free
  lightnvm: pblk: use vfree to free metadata on error path
  lightnvm: pblk: extend line wp balance check

Heiner Litz (1):
  lightnvm: pblk: fix race condition on GC

Javier González (1):
  lightnvm: pblk: prevent stall due to wb threshold

Masahiro Yamada (1):
  lightnvm: pblk: fix TRACE_INCLUDE_PATH

 drivers/lightnvm/pblk-core.c     |  8 ++--
 drivers/lightnvm/pblk-gc.c       | 22 +++++++----
 drivers/lightnvm/pblk-init.c     |  4 +-
 drivers/lightnvm/pblk-map.c      |  1 +
 drivers/lightnvm/pblk-rb.c       | 26 ++++++++++---
 drivers/lightnvm/pblk-recovery.c | 64 +++++++++++++++++++++-----------
 drivers/lightnvm/pblk-rl.c       |  5 +--
 drivers/lightnvm/pblk-trace.h    |  2 +-
 drivers/lightnvm/pblk-write.c    |  1 +
 drivers/lightnvm/pblk.h          | 17 +++------
 10 files changed, 93 insertions(+), 57 deletions(-)

-- 
2.19.1


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

* [GIT PULL 1/8] lightnvm: pblk: stop taking the free lock in in pblk_lines_free
  2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
@ 2019-02-11 12:25 ` Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 2/8] lightnvm: pblk: use vfree to free metadata on error path Matias Bjørling
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Matias Bjørling @ 2019-02-11 12:25 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-kernel, Hans Holmberg, Matias Bjørling

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

pblk_line_meta_free might sleep (it can end up calling vfree, depending
on how we allocate lba lists), and this can lead to a BUG()
if we wake up on a different cpu and release the lock.

As there is no point of grabbing the free lock when pblk has shut down,
remove the lock.

Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
Signed-off-by: Matias Bjørling <mb@lightnvm.io>
---
 drivers/lightnvm/pblk-init.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index f9a3e47b6a93..eb0135c77805 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -584,14 +584,12 @@ static void pblk_lines_free(struct pblk *pblk)
 	struct pblk_line *line;
 	int i;
 
-	spin_lock(&l_mg->free_lock);
 	for (i = 0; i < l_mg->nr_lines; i++) {
 		line = &pblk->lines[i];
 
 		pblk_line_free(line);
 		pblk_line_meta_free(l_mg, line);
 	}
-	spin_unlock(&l_mg->free_lock);
 
 	pblk_line_mg_free(pblk);
 
-- 
2.19.1


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

* [GIT PULL 2/8] lightnvm: pblk: use vfree to free metadata on error path
  2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 1/8] lightnvm: pblk: stop taking the free lock in in pblk_lines_free Matias Bjørling
@ 2019-02-11 12:25 ` Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 3/8] lightnvm: Use u64 instead of __le64 for CPU visible side Matias Bjørling
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Matias Bjørling @ 2019-02-11 12:25 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-kernel, Hans Holmberg, Matias Bjørling

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

As chunk metadata is allocated using vmalloc, we need to free it
using vfree.

Fixes: 090ee26fd512 ("lightnvm: use internal allocation for chunk log page")
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
Signed-off-by: Matias Bjørling <mb@lightnvm.io>
---
 drivers/lightnvm/pblk-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 1ff165351180..1b5ff51faa63 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -141,7 +141,7 @@ struct nvm_chk_meta *pblk_get_chunk_meta(struct pblk *pblk)
 
 	ret = nvm_get_chunk_meta(dev, ppa, geo->all_chunks, meta);
 	if (ret) {
-		kfree(meta);
+		vfree(meta);
 		return ERR_PTR(-EIO);
 	}
 
-- 
2.19.1


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

* [GIT PULL 3/8] lightnvm: Use u64 instead of __le64 for CPU visible side
  2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 1/8] lightnvm: pblk: stop taking the free lock in in pblk_lines_free Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 2/8] lightnvm: pblk: use vfree to free metadata on error path Matias Bjørling
@ 2019-02-11 12:25 ` Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 4/8] lightnvm: pblk: Switch to use new generic UUID API Matias Bjørling
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Matias Bjørling @ 2019-02-11 12:25 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-kernel, Andy Shevchenko, Matias Bjørling

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Sparse complains about using strict data types:

drivers/lightnvm/pblk-read.c:254:43: warning: incorrect type in assignment (different base types)
drivers/lightnvm/pblk-read.c:254:43:    expected restricted __le64 <noident>
drivers/lightnvm/pblk-read.c:254:43:    got unsigned long long [unsigned] [usertype] <noident>
drivers/lightnvm/pblk-read.c:255:29: warning: cast from restricted __le64
drivers/lightnvm/pblk-read.c:268:29: warning: cast from restricted __le64
drivers/lightnvm/pblk-read.c:328:41: warning: incorrect type in assignment (different base types)
drivers/lightnvm/pblk-read.c:328:41:    expected restricted __le64 <noident>
drivers/lightnvm/pblk-read.c:328:41:    got unsigned long long [unsigned] [usertype] <noident>

In the code it seems explicit that lba_list_mem and lba_list_media members
of struct pblk_pr_ctx are used on CPU side, which means they should not be
of strict types.

Change types of lba_list_mem and lba_list_media members to be u64.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Javier González <javier@javigon.com>
Signed-off-by: Matias Bjørling <mb@lightnvm.io>
---
 drivers/lightnvm/pblk.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index 85e38ed62f85..0dd697ea201e 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -131,8 +131,8 @@ struct pblk_pr_ctx {
 	unsigned int bio_init_idx;
 	void *ppa_ptr;
 	dma_addr_t dma_ppa_list;
-	__le64 lba_list_mem[NVM_MAX_VLBA];
-	__le64 lba_list_media[NVM_MAX_VLBA];
+	u64 lba_list_mem[NVM_MAX_VLBA];
+	u64 lba_list_media[NVM_MAX_VLBA];
 };
 
 /* Pad context */
-- 
2.19.1


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

* [GIT PULL 4/8] lightnvm: pblk: Switch to use new generic UUID API
  2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
                   ` (2 preceding siblings ...)
  2019-02-11 12:25 ` [GIT PULL 3/8] lightnvm: Use u64 instead of __le64 for CPU visible side Matias Bjørling
@ 2019-02-11 12:25 ` Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 5/8] lightnvm: pblk: fix TRACE_INCLUDE_PATH Matias Bjørling
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Matias Bjørling @ 2019-02-11 12:25 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-kernel, Andy Shevchenko, Matias Bjørling

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

There are new types and helpers that are supposed to be used in new code.

As a preparation to get rid of legacy types and API functions do
the conversion here.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Javier González <javier@javigon.com>
Signed-off-by: Matias Bjørling <mb@lightnvm.io>
---
 drivers/lightnvm/pblk-core.c     |  5 +++--
 drivers/lightnvm/pblk-init.c     |  2 +-
 drivers/lightnvm/pblk-recovery.c |  8 +++++---
 drivers/lightnvm/pblk.h          | 10 +---------
 4 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 1b5ff51faa63..2a9e9facf44f 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1065,7 +1065,7 @@ static int pblk_line_init_metadata(struct pblk *pblk, struct pblk_line *line,
 	bitmap_set(line->lun_bitmap, 0, lm->lun_bitmap_len);
 
 	smeta_buf->header.identifier = cpu_to_le32(PBLK_MAGIC);
-	memcpy(smeta_buf->header.uuid, pblk->instance_uuid, 16);
+	guid_copy((guid_t *)&smeta_buf->header.uuid, &pblk->instance_uuid);
 	smeta_buf->header.id = cpu_to_le32(line->id);
 	smeta_buf->header.type = cpu_to_le16(line->type);
 	smeta_buf->header.version_major = SMETA_VERSION_MAJOR;
@@ -1874,7 +1874,8 @@ void pblk_line_close_meta(struct pblk *pblk, struct pblk_line *line)
 
 	if (le32_to_cpu(emeta_buf->header.identifier) != PBLK_MAGIC) {
 		emeta_buf->header.identifier = cpu_to_le32(PBLK_MAGIC);
-		memcpy(emeta_buf->header.uuid, pblk->instance_uuid, 16);
+		guid_copy((guid_t *)&emeta_buf->header.uuid,
+							&pblk->instance_uuid);
 		emeta_buf->header.id = cpu_to_le32(line->id);
 		emeta_buf->header.type = cpu_to_le16(line->type);
 		emeta_buf->header.version_major = EMETA_VERSION_MAJOR;
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index eb0135c77805..8b643d0bffae 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -130,7 +130,7 @@ static int pblk_l2p_recover(struct pblk *pblk, bool factory_init)
 	struct pblk_line *line = NULL;
 
 	if (factory_init) {
-		pblk_setup_uuid(pblk);
+		guid_gen(&pblk->instance_uuid);
 	} else {
 		line = pblk_recov_l2p(pblk);
 		if (IS_ERR(line)) {
diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
index 5ee20da7bdb3..6761d2afa4d0 100644
--- a/drivers/lightnvm/pblk-recovery.c
+++ b/drivers/lightnvm/pblk-recovery.c
@@ -703,11 +703,13 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
 
 		/* The first valid instance uuid is used for initialization */
 		if (!valid_uuid) {
-			memcpy(pblk->instance_uuid, smeta_buf->header.uuid, 16);
+			guid_copy(&pblk->instance_uuid,
+				  (guid_t *)&smeta_buf->header.uuid);
 			valid_uuid = 1;
 		}
 
-		if (memcmp(pblk->instance_uuid, smeta_buf->header.uuid, 16)) {
+		if (!guid_equal(&pblk->instance_uuid,
+				(guid_t *)&smeta_buf->header.uuid)) {
 			pblk_debug(pblk, "ignore line %u due to uuid mismatch\n",
 					i);
 			continue;
@@ -737,7 +739,7 @@ struct pblk_line *pblk_recov_l2p(struct pblk *pblk)
 	}
 
 	if (!found_lines) {
-		pblk_setup_uuid(pblk);
+		guid_gen(&pblk->instance_uuid);
 
 		spin_lock(&l_mg->free_lock);
 		WARN_ON_ONCE(!test_and_clear_bit(meta_line,
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index 0dd697ea201e..72ae8755764e 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -646,7 +646,7 @@ struct pblk {
 
 	int sec_per_write;
 
-	unsigned char instance_uuid[16];
+	guid_t instance_uuid;
 
 	/* Persistent write amplification counters, 4kb sector I/Os */
 	atomic64_t user_wa;		/* Sectors written by user */
@@ -1360,14 +1360,6 @@ static inline unsigned int pblk_get_secs(struct bio *bio)
 	return  bio->bi_iter.bi_size / PBLK_EXPOSED_PAGE_SIZE;
 }
 
-static inline void pblk_setup_uuid(struct pblk *pblk)
-{
-	uuid_le uuid;
-
-	uuid_le_gen(&uuid);
-	memcpy(pblk->instance_uuid, uuid.b, 16);
-}
-
 static inline char *pblk_disk_name(struct pblk *pblk)
 {
 	struct gendisk *disk = pblk->disk;
-- 
2.19.1


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

* [GIT PULL 5/8] lightnvm: pblk: fix TRACE_INCLUDE_PATH
  2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
                   ` (3 preceding siblings ...)
  2019-02-11 12:25 ` [GIT PULL 4/8] lightnvm: pblk: Switch to use new generic UUID API Matias Bjørling
@ 2019-02-11 12:25 ` Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 6/8] lightnvm: pblk: extend line wp balance check Matias Bjørling
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Matias Bjørling @ 2019-02-11 12:25 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-kernel, Masahiro Yamada, Matias Bjørling

From: Masahiro Yamada <yamada.masahiro@socionext.com>

As the comment block in include/trace/define_trace.h says,
TRACE_INCLUDE_PATH should be a relative path to the define_trace.h

../../drivers/lightnvm is the correct relative path.

../../../drivers/lightnvm is working by coincidence because the top
Makefile adds -I$(srctree)/arch/$(SRCARCH)/include as a header
search path, but we should not rely on it.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
Signed-off-by: Matias Bjørling <mb@lightnvm.io>
---
 drivers/lightnvm/pblk-trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/lightnvm/pblk-trace.h b/drivers/lightnvm/pblk-trace.h
index 679e5c458ca6..9534503b69d9 100644
--- a/drivers/lightnvm/pblk-trace.h
+++ b/drivers/lightnvm/pblk-trace.h
@@ -139,7 +139,7 @@ TRACE_EVENT(pblk_state,
 /* This part must be outside protection */
 
 #undef TRACE_INCLUDE_PATH
-#define TRACE_INCLUDE_PATH ../../../drivers/lightnvm
+#define TRACE_INCLUDE_PATH ../../drivers/lightnvm
 #undef TRACE_INCLUDE_FILE
 #define TRACE_INCLUDE_FILE pblk-trace
 #include <trace/define_trace.h>
-- 
2.19.1


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

* [GIT PULL 6/8] lightnvm: pblk: extend line wp balance check
  2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
                   ` (4 preceding siblings ...)
  2019-02-11 12:25 ` [GIT PULL 5/8] lightnvm: pblk: fix TRACE_INCLUDE_PATH Matias Bjørling
@ 2019-02-11 12:25 ` Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 7/8] lightnvm: pblk: prevent stall due to wb threshold Matias Bjørling
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Matias Bjørling @ 2019-02-11 12:25 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-kernel, Hans Holmberg, Matias Bjørling

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

pblk stripes writes of minimal write size across all non-offline chunks
in a line, which means that the maximum write pointer delta should not
exceed the minimal write size.

Extend the line write pointer balance check to cover this case, and
ignore the offline chunk wps.

This will render us a warning during recovery if something unexpected
has happened to the chunk write pointers (i.e. powerloss,  a spurious
chunk reset, ..).

Reported-by: Zhoujie Wu <zjwu@marvell.com>
Tested-by: Zhoujie Wu <zjwu@marvell.com>
Reviewed-by: Javier González <javier@javigon.com>
Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
Signed-off-by: Matias Bjørling <mb@lightnvm.io>
---
 drivers/lightnvm/pblk-recovery.c | 56 ++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
index 6761d2afa4d0..d86f580036d3 100644
--- a/drivers/lightnvm/pblk-recovery.c
+++ b/drivers/lightnvm/pblk-recovery.c
@@ -302,35 +302,55 @@ static int pblk_pad_distance(struct pblk *pblk, struct pblk_line *line)
 	return (distance > line->left_msecs) ? line->left_msecs : distance;
 }
 
-static int pblk_line_wp_is_unbalanced(struct pblk *pblk,
-				      struct pblk_line *line)
+/* Return a chunk belonging to a line by stripe(write order) index */
+static struct nvm_chk_meta *pblk_get_stripe_chunk(struct pblk *pblk,
+						  struct pblk_line *line,
+						  int index)
 {
 	struct nvm_tgt_dev *dev = pblk->dev;
 	struct nvm_geo *geo = &dev->geo;
-	struct pblk_line_meta *lm = &pblk->lm;
 	struct pblk_lun *rlun;
-	struct nvm_chk_meta *chunk;
 	struct ppa_addr ppa;
-	u64 line_wp;
-	int pos, i;
+	int pos;
 
-	rlun = &pblk->luns[0];
+	rlun = &pblk->luns[index];
 	ppa = rlun->bppa;
 	pos = pblk_ppa_to_pos(geo, ppa);
-	chunk = &line->chks[pos];
 
-	line_wp = chunk->wp;
+	return &line->chks[pos];
+}
 
-	for (i = 1; i < lm->blk_per_line; i++) {
-		rlun = &pblk->luns[i];
-		ppa = rlun->bppa;
-		pos = pblk_ppa_to_pos(geo, ppa);
-		chunk = &line->chks[pos];
+static int pblk_line_wps_are_unbalanced(struct pblk *pblk,
+				      struct pblk_line *line)
+{
+	struct pblk_line_meta *lm = &pblk->lm;
+	int blk_in_line = lm->blk_per_line;
+	struct nvm_chk_meta *chunk;
+	u64 max_wp, min_wp;
+	int i;
 
-		if (chunk->wp > line_wp)
+	i = find_first_zero_bit(line->blk_bitmap, blk_in_line);
+
+	/* If there is one or zero good chunks in the line,
+	 * the write pointers can't be unbalanced.
+	 */
+	if (i >= (blk_in_line - 1))
+		return 0;
+
+	chunk = pblk_get_stripe_chunk(pblk, line, i);
+	max_wp = chunk->wp;
+	if (max_wp > pblk->max_write_pgs)
+		min_wp = max_wp - pblk->max_write_pgs;
+	else
+		min_wp = 0;
+
+	i = find_next_zero_bit(line->blk_bitmap, blk_in_line, i + 1);
+	while (i < blk_in_line) {
+		chunk = pblk_get_stripe_chunk(pblk, line, i);
+		if (chunk->wp > max_wp || chunk->wp < min_wp)
 			return 1;
-		else if (chunk->wp < line_wp)
-			line_wp = chunk->wp;
+
+		i = find_next_zero_bit(line->blk_bitmap, blk_in_line, i + 1);
 	}
 
 	return 0;
@@ -356,7 +376,7 @@ static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
 	int ret;
 	u64 left_ppas = pblk_sec_in_open_line(pblk, line) - lm->smeta_sec;
 
-	if (pblk_line_wp_is_unbalanced(pblk, line))
+	if (pblk_line_wps_are_unbalanced(pblk, line))
 		pblk_warn(pblk, "recovering unbalanced line (%d)\n", line->id);
 
 	ppa_list = p.ppa_list;
-- 
2.19.1


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

* [GIT PULL 7/8] lightnvm: pblk: prevent stall due to wb threshold
  2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
                   ` (5 preceding siblings ...)
  2019-02-11 12:25 ` [GIT PULL 6/8] lightnvm: pblk: extend line wp balance check Matias Bjørling
@ 2019-02-11 12:25 ` Matias Bjørling
  2019-02-11 12:25 ` [GIT PULL 8/8] lightnvm: pblk: fix race condition on GC Matias Bjørling
  2019-02-11 15:22 ` [GIT PULL 0/8] lightnvm updates for 5.1 Jens Axboe
  8 siblings, 0 replies; 10+ messages in thread
From: Matias Bjørling @ 2019-02-11 12:25 UTC (permalink / raw)
  To: axboe
  Cc: linux-block, linux-kernel, Javier González, Matias Bjørling

From: Javier González <javier@javigon.com>

In order to respect mw_cuinits, pblk's write buffer maintains a
backpointer to protect data not yet persisted; when writing to the write
buffer, this backpointer defines a threshold that pblk's rate-limiter
enforces.

On small PU configurations, the following scenarios might take place: (i)
the threshold is larger than the write buffer and (ii) the threshold is
smaller than the write buffer, but larger than the maximun allowed
split bio - 256KB at this moment (Note that writes are not always
split - we only do this when we the size of the buffer is smaller
than the buffer). In both cases, pblk's rate-limiter prevents the I/O to
be written to the buffer, thus stalling.

This patch fixes the original backpointer implementation by considering
the threshold both on buffer creation and on the rate-limiters path,
when bio_split is triggered (case (ii) above).

Fixes: 766c8ceb16fc ("lightnvm: pblk: guarantee that backpointer is respected on writer stall")
Signed-off-by: Javier González <javier@javigon.com>
Reviewed-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
Signed-off-by: Matias Bjørling <mb@lightnvm.io>
---
 drivers/lightnvm/pblk-rb.c | 25 +++++++++++++++++++------
 drivers/lightnvm/pblk-rl.c |  5 ++---
 drivers/lightnvm/pblk.h    |  2 +-
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
index d4ca8c64ee0f..a6133b50ed9c 100644
--- a/drivers/lightnvm/pblk-rb.c
+++ b/drivers/lightnvm/pblk-rb.c
@@ -45,10 +45,23 @@ void pblk_rb_free(struct pblk_rb *rb)
 /*
  * pblk_rb_calculate_size -- calculate the size of the write buffer
  */
-static unsigned int pblk_rb_calculate_size(unsigned int nr_entries)
+static unsigned int pblk_rb_calculate_size(unsigned int nr_entries,
+					   unsigned int threshold)
 {
-	/* Alloc a write buffer that can at least fit 128 entries */
-	return (1 << max(get_count_order(nr_entries), 7));
+	unsigned int thr_sz = 1 << (get_count_order(threshold + NVM_MAX_VLBA));
+	unsigned int max_sz = max(thr_sz, nr_entries);
+	unsigned int max_io;
+
+	/* Alloc a write buffer that can (i) fit at least two split bios
+	 * (considering max I/O size NVM_MAX_VLBA, and (ii) guarantee that the
+	 * threshold will be respected
+	 */
+	max_io = (1 << max((int)(get_count_order(max_sz)),
+				(int)(get_count_order(NVM_MAX_VLBA << 1))));
+	if ((threshold + NVM_MAX_VLBA) >= max_io)
+		max_io <<= 1;
+
+	return max_io;
 }
 
 /*
@@ -67,12 +80,12 @@ int pblk_rb_init(struct pblk_rb *rb, unsigned int size, unsigned int threshold,
 	unsigned int alloc_order, order, iter;
 	unsigned int nr_entries;
 
-	nr_entries = pblk_rb_calculate_size(size);
+	nr_entries = pblk_rb_calculate_size(size, threshold);
 	entries = vzalloc(array_size(nr_entries, sizeof(struct pblk_rb_entry)));
 	if (!entries)
 		return -ENOMEM;
 
-	power_size = get_count_order(size);
+	power_size = get_count_order(nr_entries);
 	power_seg_sz = get_count_order(seg_size);
 
 	down_write(&pblk_rb_lock);
@@ -149,7 +162,7 @@ int pblk_rb_init(struct pblk_rb *rb, unsigned int size, unsigned int threshold,
 	 * Initialize rate-limiter, which controls access to the write buffer
 	 * by user and GC I/O
 	 */
-	pblk_rl_init(&pblk->rl, rb->nr_entries);
+	pblk_rl_init(&pblk->rl, rb->nr_entries, threshold);
 
 	return 0;
 }
diff --git a/drivers/lightnvm/pblk-rl.c b/drivers/lightnvm/pblk-rl.c
index 76116d5f78e4..b014957dde0b 100644
--- a/drivers/lightnvm/pblk-rl.c
+++ b/drivers/lightnvm/pblk-rl.c
@@ -207,7 +207,7 @@ void pblk_rl_free(struct pblk_rl *rl)
 	del_timer(&rl->u_timer);
 }
 
-void pblk_rl_init(struct pblk_rl *rl, int budget)
+void pblk_rl_init(struct pblk_rl *rl, int budget, int threshold)
 {
 	struct pblk *pblk = container_of(rl, struct pblk, rl);
 	struct nvm_tgt_dev *dev = pblk->dev;
@@ -217,7 +217,6 @@ void pblk_rl_init(struct pblk_rl *rl, int budget)
 	int sec_meta, blk_meta;
 	unsigned int rb_windows;
 
-
 	/* Consider sectors used for metadata */
 	sec_meta = (lm->smeta_sec + lm->emeta_sec[0]) * l_mg->nr_free_lines;
 	blk_meta = DIV_ROUND_UP(sec_meta, geo->clba);
@@ -234,7 +233,7 @@ void pblk_rl_init(struct pblk_rl *rl, int budget)
 	/* To start with, all buffer is available to user I/O writers */
 	rl->rb_budget = budget;
 	rl->rb_user_max = budget;
-	rl->rb_max_io = budget >> 1;
+	rl->rb_max_io = threshold ? (budget - threshold) : (budget - 1);
 	rl->rb_gc_max = 0;
 	rl->rb_state = PBLK_RL_HIGH;
 
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index 72ae8755764e..a6386d5acd73 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -924,7 +924,7 @@ int pblk_gc_sysfs_force(struct pblk *pblk, int force);
 /*
  * pblk rate limiter
  */
-void pblk_rl_init(struct pblk_rl *rl, int budget);
+void pblk_rl_init(struct pblk_rl *rl, int budget, int threshold);
 void pblk_rl_free(struct pblk_rl *rl);
 void pblk_rl_update_rates(struct pblk_rl *rl);
 int pblk_rl_high_thrs(struct pblk_rl *rl);
-- 
2.19.1


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

* [GIT PULL 8/8] lightnvm: pblk: fix race condition on GC
  2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
                   ` (6 preceding siblings ...)
  2019-02-11 12:25 ` [GIT PULL 7/8] lightnvm: pblk: prevent stall due to wb threshold Matias Bjørling
@ 2019-02-11 12:25 ` Matias Bjørling
  2019-02-11 15:22 ` [GIT PULL 0/8] lightnvm updates for 5.1 Jens Axboe
  8 siblings, 0 replies; 10+ messages in thread
From: Matias Bjørling @ 2019-02-11 12:25 UTC (permalink / raw)
  To: axboe; +Cc: linux-block, linux-kernel, Heiner Litz, Matias Bjørling

From: Heiner Litz <hlitz@ucsc.edu>

This patch fixes a race condition where a write is mapped to the last
sectors of a line. The write is synced to the device but the L2P is not
updated yet. When the line is garbage collected before the L2P update
is performed, the sectors are ignored by the GC logic and the line is
freed before all sectors are moved. When the L2P is finally updated, it
contains a mapping to a freed line, subsequent reads of the
corresponding LBAs fail.

This patch introduces a per line counter specifying the number of
sectors that are synced to the device but have not been updated in the
L2P. Lines with a counter of greater than zero will not be selected
for GC.

Signed-off-by: Heiner Litz <hlitz@ucsc.edu>
Reviewed-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
Reviewed-by: Javier González <javier@javigon.com>
Signed-off-by: Matias Bjørling <mb@lightnvm.io>
---
 drivers/lightnvm/pblk-core.c  |  1 +
 drivers/lightnvm/pblk-gc.c    | 22 ++++++++++++++--------
 drivers/lightnvm/pblk-map.c   |  1 +
 drivers/lightnvm/pblk-rb.c    |  1 +
 drivers/lightnvm/pblk-write.c |  1 +
 drivers/lightnvm/pblk.h       |  1 +
 6 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
index 2a9e9facf44f..6ca868868fee 100644
--- a/drivers/lightnvm/pblk-core.c
+++ b/drivers/lightnvm/pblk-core.c
@@ -1278,6 +1278,7 @@ static int pblk_line_prepare(struct pblk *pblk, struct pblk_line *line)
 	spin_unlock(&line->lock);
 
 	kref_init(&line->ref);
+	atomic_set(&line->sec_to_update, 0);
 
 	return 0;
 }
diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
index 2fa118c8eb71..26a52ea7ec45 100644
--- a/drivers/lightnvm/pblk-gc.c
+++ b/drivers/lightnvm/pblk-gc.c
@@ -365,16 +365,22 @@ static struct pblk_line *pblk_gc_get_victim_line(struct pblk *pblk,
 						 struct list_head *group_list)
 {
 	struct pblk_line *line, *victim;
-	int line_vsc, victim_vsc;
+	unsigned int line_vsc = ~0x0L, victim_vsc = ~0x0L;
 
 	victim = list_first_entry(group_list, struct pblk_line, list);
+
 	list_for_each_entry(line, group_list, list) {
-		line_vsc = le32_to_cpu(*line->vsc);
-		victim_vsc = le32_to_cpu(*victim->vsc);
-		if (line_vsc < victim_vsc)
+		if (!atomic_read(&line->sec_to_update))
+			line_vsc = le32_to_cpu(*line->vsc);
+		if (line_vsc < victim_vsc) {
 			victim = line;
+			victim_vsc = le32_to_cpu(*victim->vsc);
+		}
 	}
 
+	if (victim_vsc == ~0x0)
+		return NULL;
+
 	return victim;
 }
 
@@ -448,12 +454,12 @@ static void pblk_gc_run(struct pblk *pblk)
 
 	do {
 		spin_lock(&l_mg->gc_lock);
-		if (list_empty(group_list)) {
-			spin_unlock(&l_mg->gc_lock);
-			break;
-		}
 
 		line = pblk_gc_get_victim_line(pblk, group_list);
+		if (!line) {
+			spin_unlock(&l_mg->gc_lock);
+			break;
+		}
 
 		spin_lock(&line->lock);
 		WARN_ON(line->state != PBLK_LINESTATE_CLOSED);
diff --git a/drivers/lightnvm/pblk-map.c b/drivers/lightnvm/pblk-map.c
index 79df583ea709..7fbc99b60cac 100644
--- a/drivers/lightnvm/pblk-map.c
+++ b/drivers/lightnvm/pblk-map.c
@@ -73,6 +73,7 @@ static int pblk_map_page_data(struct pblk *pblk, unsigned int sentry,
 		 */
 		if (i < valid_secs) {
 			kref_get(&line->ref);
+			atomic_inc(&line->sec_to_update);
 			w_ctx = pblk_rb_w_ctx(&pblk->rwb, sentry + i);
 			w_ctx->ppa = ppa_list[i];
 			meta->lba = cpu_to_le64(w_ctx->lba);
diff --git a/drivers/lightnvm/pblk-rb.c b/drivers/lightnvm/pblk-rb.c
index a6133b50ed9c..03c241b340ea 100644
--- a/drivers/lightnvm/pblk-rb.c
+++ b/drivers/lightnvm/pblk-rb.c
@@ -260,6 +260,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int to_update)
 							entry->cacheline);
 
 		line = pblk_ppa_to_line(pblk, w_ctx->ppa);
+		atomic_dec(&line->sec_to_update);
 		kref_put(&line->ref, pblk_line_put);
 		clean_wctx(w_ctx);
 		rb->l2p_update = pblk_rb_ptr_wrap(rb, rb->l2p_update, 1);
diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
index 06d56deb645d..6593deab52da 100644
--- a/drivers/lightnvm/pblk-write.c
+++ b/drivers/lightnvm/pblk-write.c
@@ -177,6 +177,7 @@ static void pblk_prepare_resubmit(struct pblk *pblk, unsigned int sentry,
 		 * re-map these entries
 		 */
 		line = pblk_ppa_to_line(pblk, w_ctx->ppa);
+		atomic_dec(&line->sec_to_update);
 		kref_put(&line->ref, pblk_line_put);
 	}
 	spin_unlock(&pblk->trans_lock);
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index a6386d5acd73..ac3ab778e976 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -487,6 +487,7 @@ struct pblk_line {
 	__le32 *vsc;			/* Valid sector count in line */
 
 	struct kref ref;		/* Write buffer L2P references */
+	atomic_t sec_to_update;         /* Outstanding L2P updates to ppa */
 
 	struct pblk_w_err_gc *w_err_gc;	/* Write error gc recovery metadata */
 
-- 
2.19.1


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

* Re: [GIT PULL 0/8] lightnvm updates for 5.1
  2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
                   ` (7 preceding siblings ...)
  2019-02-11 12:25 ` [GIT PULL 8/8] lightnvm: pblk: fix race condition on GC Matias Bjørling
@ 2019-02-11 15:22 ` Jens Axboe
  8 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2019-02-11 15:22 UTC (permalink / raw)
  To: Matias Bjørling; +Cc: linux-block, linux-kernel

On 2/11/19 5:25 AM, Matias Bjørling wrote:
> Hi Jens,
> 
> Would you please pick up the following patches for 5.1?
> 
> It is a bunch of misc patches this time. A couple of fixes and cleanups.
> 
> Andy Shevchenko (2):
>   lightnvm: Use u64 instead of __le64 for CPU visible side
>   lightnvm: pblk: Switch to use new generic UUID API
> 
> Hans Holmberg (3):
>   lightnvm: pblk: stop taking the free lock in in pblk_lines_free
>   lightnvm: pblk: use vfree to free metadata on error path
>   lightnvm: pblk: extend line wp balance check
> 
> Heiner Litz (1):
>   lightnvm: pblk: fix race condition on GC
> 
> Javier González (1):
>   lightnvm: pblk: prevent stall due to wb threshold
> 
> Masahiro Yamada (1):
>   lightnvm: pblk: fix TRACE_INCLUDE_PATH
> 
>  drivers/lightnvm/pblk-core.c     |  8 ++--
>  drivers/lightnvm/pblk-gc.c       | 22 +++++++----
>  drivers/lightnvm/pblk-init.c     |  4 +-
>  drivers/lightnvm/pblk-map.c      |  1 +
>  drivers/lightnvm/pblk-rb.c       | 26 ++++++++++---
>  drivers/lightnvm/pblk-recovery.c | 64 +++++++++++++++++++++-----------
>  drivers/lightnvm/pblk-rl.c       |  5 +--
>  drivers/lightnvm/pblk-trace.h    |  2 +-
>  drivers/lightnvm/pblk-write.c    |  1 +
>  drivers/lightnvm/pblk.h          | 17 +++------
>  10 files changed, 93 insertions(+), 57 deletions(-)

Applied, thanks.

-- 
Jens Axboe


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

end of thread, other threads:[~2019-02-11 15:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-11 12:25 [GIT PULL 0/8] lightnvm updates for 5.1 Matias Bjørling
2019-02-11 12:25 ` [GIT PULL 1/8] lightnvm: pblk: stop taking the free lock in in pblk_lines_free Matias Bjørling
2019-02-11 12:25 ` [GIT PULL 2/8] lightnvm: pblk: use vfree to free metadata on error path Matias Bjørling
2019-02-11 12:25 ` [GIT PULL 3/8] lightnvm: Use u64 instead of __le64 for CPU visible side Matias Bjørling
2019-02-11 12:25 ` [GIT PULL 4/8] lightnvm: pblk: Switch to use new generic UUID API Matias Bjørling
2019-02-11 12:25 ` [GIT PULL 5/8] lightnvm: pblk: fix TRACE_INCLUDE_PATH Matias Bjørling
2019-02-11 12:25 ` [GIT PULL 6/8] lightnvm: pblk: extend line wp balance check Matias Bjørling
2019-02-11 12:25 ` [GIT PULL 7/8] lightnvm: pblk: prevent stall due to wb threshold Matias Bjørling
2019-02-11 12:25 ` [GIT PULL 8/8] lightnvm: pblk: fix race condition on GC Matias Bjørling
2019-02-11 15:22 ` [GIT PULL 0/8] lightnvm updates for 5.1 Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).