diff mbox series

[v2,4/7] lightnvm: pblk: set conservative threshold for user writes

Message ID 20181105122610.1555-5-hans.ml.holmberg@owltronix.com (mailing list archive)
State New, archived
Headers show
Series PBLK Bugfixes and cleanups | expand

Commit Message

Hans Holmberg Nov. 5, 2018, 12:26 p.m. UTC 
From: Hans Holmberg <hans.holmberg@cnexlabs.com>

In a worst-case scenario (random writes), OP% of sectors
in each line will be invalid, and we will then need
to move data out of 100/OP% lines to free a single line.

So, to prevent the possibility of running out of lines,
temporarily block user writes when there is less than
100/OP% free lines.

Also ensure that pblk creation does not produce instances
with insufficient over provisioning.

Insufficient over-provising is not a problem on real hardware,
but often an issue when running QEMU simulations (with few lines).
100 lines is enough to create a sane instance with the standard
(11%) over provisioning.

Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
---
 drivers/lightnvm/pblk-init.c | 43 ++++++++++++++++++++++++------------
 drivers/lightnvm/pblk-rl.c   |  5 ++---
 drivers/lightnvm/pblk.h      | 12 +++++++++-
 3 files changed, 42 insertions(+), 18 deletions(-)

Comments

kernel test robot Nov. 5, 2018, 9:15 p.m. UTC | #1 
Hi Hans,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc1 next-20181105]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hans-Holmberg/PBLK-Bugfixes-and-cleanups/20181106-022237
config: i386-randconfig-sb0-11060349 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/lightnvm/pblk-init.o: In function `pblk_set_provision':
>> drivers/lightnvm/pblk-init.c:668: undefined reference to `__udivdi3'

vim +668 drivers/lightnvm/pblk-init.c

   637	
   638	static int pblk_set_provision(struct pblk *pblk, long nr_free_chks)
   639	{
   640		struct nvm_tgt_dev *dev = pblk->dev;
   641		struct pblk_line_mgmt *l_mg = &pblk->l_mg;
   642		struct pblk_line_meta *lm = &pblk->lm;
   643		struct nvm_geo *geo = &dev->geo;
   644		sector_t provisioned, minimum;
   645		int sec_meta, blk_meta;
   646	
   647		if (geo->op == NVM_TARGET_DEFAULT_OP)
   648			pblk->op = PBLK_DEFAULT_OP;
   649		else
   650			pblk->op = geo->op;
   651	
   652		minimum = pblk_get_min_chks(pblk);
   653		provisioned = nr_free_chks;
   654		provisioned *= (100 - pblk->op);
   655		sector_div(provisioned, 100);
   656	
   657		if ((nr_free_chks - provisioned) < minimum) {
   658			if (geo->op != NVM_TARGET_DEFAULT_OP) {
   659				pblk_err(pblk, "OP too small to create a sane instance\n");
   660				return -EINTR;
   661			}
   662	
   663			/* If the user did not specify an OP value, and PBLK_DEFAULT_OP
   664			 * is not enough, calculate and set sane value
   665			 */
   666	
   667			provisioned = nr_free_chks - minimum;
 > 668			pblk->op =  (100 * minimum) / nr_free_chks;
   669			pblk_info(pblk, "Default OP insufficient, adjusting OP to %d\n",
   670					pblk->op);
   671		}
   672	
   673		pblk->op_blks = nr_free_chks - provisioned;
   674	
   675		/* Internally pblk manages all free blocks, but all calculations based
   676		 * on user capacity consider only provisioned blocks
   677		 */
   678		pblk->rl.total_blocks = nr_free_chks;
   679		pblk->rl.nr_secs = nr_free_chks * geo->clba;
   680	
   681		/* Consider sectors used for metadata */
   682		sec_meta = (lm->smeta_sec + lm->emeta_sec[0]) * l_mg->nr_free_lines;
   683		blk_meta = DIV_ROUND_UP(sec_meta, geo->clba);
   684	
   685		pblk->capacity = (provisioned - blk_meta) * geo->clba;
   686	
   687		atomic_set(&pblk->rl.free_blocks, nr_free_chks);
   688		atomic_set(&pblk->rl.free_user_blocks, nr_free_chks);
   689	
   690		return 0;
   691	}
   692	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Nov. 5, 2018, 11:09 p.m. UTC | #2 
Hi Hans,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc1 next-20181105]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Hans-Holmberg/PBLK-Bugfixes-and-cleanups/20181106-022237
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/lightnvm/pblk-init.o: In function `pblk_lines_init':
>> pblk-init.c:(.text+0x1c94): undefined reference to `__aeabi_uldivmod'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c
index 13822594647c..8b89bb26b0f1 100644
--- a/drivers/lightnvm/pblk-init.c
+++ b/drivers/lightnvm/pblk-init.c
@@ -635,13 +635,13 @@  static unsigned int calc_emeta_len(struct pblk *pblk)
 	return (lm->emeta_len[1] + lm->emeta_len[2] + lm->emeta_len[3]);
 }
 
-static void pblk_set_provision(struct pblk *pblk, long nr_free_blks)
+static int pblk_set_provision(struct pblk *pblk, long nr_free_chks)
 {
 	struct nvm_tgt_dev *dev = pblk->dev;
 	struct pblk_line_mgmt *l_mg = &pblk->l_mg;
 	struct pblk_line_meta *lm = &pblk->lm;
 	struct nvm_geo *geo = &dev->geo;
-	sector_t provisioned;
+	sector_t provisioned, minimum;
 	int sec_meta, blk_meta;
 
 	if (geo->op == NVM_TARGET_DEFAULT_OP)
@@ -649,17 +649,34 @@  static void pblk_set_provision(struct pblk *pblk, long nr_free_blks)
 	else
 		pblk->op = geo->op;
 
-	provisioned = nr_free_blks;
+	minimum = pblk_get_min_chks(pblk);
+	provisioned = nr_free_chks;
 	provisioned *= (100 - pblk->op);
 	sector_div(provisioned, 100);
 
-	pblk->op_blks = nr_free_blks - provisioned;
+	if ((nr_free_chks - provisioned) < minimum) {
+		if (geo->op != NVM_TARGET_DEFAULT_OP) {
+			pblk_err(pblk, "OP too small to create a sane instance\n");
+			return -EINTR;
+		}
+
+		/* If the user did not specify an OP value, and PBLK_DEFAULT_OP
+		 * is not enough, calculate and set sane value
+		 */
+
+		provisioned = nr_free_chks - minimum;
+		pblk->op =  (100 * minimum) / nr_free_chks;
+		pblk_info(pblk, "Default OP insufficient, adjusting OP to %d\n",
+				pblk->op);
+	}
+
+	pblk->op_blks = nr_free_chks - provisioned;
 
 	/* Internally pblk manages all free blocks, but all calculations based
 	 * on user capacity consider only provisioned blocks
 	 */
-	pblk->rl.total_blocks = nr_free_blks;
-	pblk->rl.nr_secs = nr_free_blks * geo->clba;
+	pblk->rl.total_blocks = nr_free_chks;
+	pblk->rl.nr_secs = nr_free_chks * geo->clba;
 
 	/* Consider sectors used for metadata */
 	sec_meta = (lm->smeta_sec + lm->emeta_sec[0]) * l_mg->nr_free_lines;
@@ -667,8 +684,10 @@  static void pblk_set_provision(struct pblk *pblk, long nr_free_blks)
 
 	pblk->capacity = (provisioned - blk_meta) * geo->clba;
 
-	atomic_set(&pblk->rl.free_blocks, nr_free_blks);
-	atomic_set(&pblk->rl.free_user_blocks, nr_free_blks);
+	atomic_set(&pblk->rl.free_blocks, nr_free_chks);
+	atomic_set(&pblk->rl.free_user_blocks, nr_free_chks);
+
+	return 0;
 }
 
 static int pblk_setup_line_meta_chk(struct pblk *pblk, struct pblk_line *line,
@@ -1025,13 +1044,9 @@  static int pblk_lines_init(struct pblk *pblk)
 								line->state);
 	}
 
-	if (!nr_free_chks) {
-		pblk_err(pblk, "too many bad blocks prevent for sane instance\n");
-		ret = -EINTR;
+	ret = pblk_set_provision(pblk, nr_free_chks);
+	if (ret)
 		goto fail_free_lines;
-	}
-
-	pblk_set_provision(pblk, nr_free_chks);
 
 	vfree(chunk_meta);
 	return 0;
diff --git a/drivers/lightnvm/pblk-rl.c b/drivers/lightnvm/pblk-rl.c
index db55a1c89997..76116d5f78e4 100644
--- a/drivers/lightnvm/pblk-rl.c
+++ b/drivers/lightnvm/pblk-rl.c
@@ -214,11 +214,10 @@  void pblk_rl_init(struct pblk_rl *rl, int budget)
 	struct nvm_geo *geo = &dev->geo;
 	struct pblk_line_mgmt *l_mg = &pblk->l_mg;
 	struct pblk_line_meta *lm = &pblk->lm;
-	int min_blocks = lm->blk_per_line * PBLK_GC_RSV_LINE;
 	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);
@@ -226,7 +225,7 @@  void pblk_rl_init(struct pblk_rl *rl, int budget)
 	rl->high = pblk->op_blks - blk_meta - lm->blk_per_line;
 	rl->high_pw = get_count_order(rl->high);
 
-	rl->rsv_blocks = min_blocks;
+	rl->rsv_blocks = pblk_get_min_chks(pblk);
 
 	/* This will always be a power-of-2 */
 	rb_windows = budget / NVM_MAX_VLBA;
diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
index f415aae600c8..e5b88a25d4d6 100644
--- a/drivers/lightnvm/pblk.h
+++ b/drivers/lightnvm/pblk.h
@@ -905,7 +905,6 @@  int pblk_recov_check_emeta(struct pblk *pblk, struct line_emeta *emeta);
 #define PBLK_GC_MAX_READERS 8	/* Max number of outstanding GC reader jobs */
 #define PBLK_GC_RQ_QD 128	/* Queue depth for inflight GC requests */
 #define PBLK_GC_L_QD 4		/* Queue depth for inflight GC lines */
-#define PBLK_GC_RSV_LINE 1	/* Reserved lines for GC */
 
 int pblk_gc_init(struct pblk *pblk);
 void pblk_gc_exit(struct pblk *pblk, bool graceful);
@@ -1370,4 +1369,15 @@  static inline char *pblk_disk_name(struct pblk *pblk)
 
 	return disk->disk_name;
 }
+
+static inline unsigned int pblk_get_min_chks(struct pblk *pblk)
+{
+	struct pblk_line_meta *lm = &pblk->lm;
+	/* In a worst-case scenario every line will have OP invalid sectors.
+	 * We will then need a minimum of 1/OP lines to free up a single line
+	 */
+
+	return DIV_ROUND_UP(100, pblk->op) * lm->blk_per_line;
+
+}
 #endif /* PBLK_H_ */