lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 3 Oct 2017 12:40:29 +0200
From:   Javier González <jg@...htnvm.io>
To:     Hans Holmberg <hans.ml.holmberg@...tronix.com>
Cc:     Matias Bjørling <mb@...htnvm.io>,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        Hans Holmberg <hans.holmberg@...xlabs.com>
Subject: Re: [PATCH 8/9] lightnvm: pblk: gc all lines in the pipeline before
 exit

> On 3 Oct 2017, at 12.05, Hans Holmberg <hans.ml.holmberg@...tronix.com> wrote:
> 
> From: Hans Holmberg <hans.holmberg@...xlabs.com>
> 
> Finish garbage collect of the lines that are in the gc pipeline
> before exiting. Ensure that all lines already in in the pipeline
> goes through, from read to write.
> 
> Do this by keeping track of how many lines are in the pipeline
> and waiting for that number to reach zero before exiting the gc
> reader task.
> 
> Since we're adding a new gc line counter, change the name of
> inflight_gc to read_inflight_gc to make the distinction clear.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg@...xlabs.com>
> ---
> drivers/lightnvm/pblk-core.c  |  3 +++
> drivers/lightnvm/pblk-gc.c    | 31 ++++++++++++++++++++++++-------
> drivers/lightnvm/pblk-sysfs.c |  2 +-
> drivers/lightnvm/pblk.h       |  5 ++++-
> 4 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index 15e1e28..e62e6eb 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -1463,6 +1463,7 @@ void pblk_line_free(struct pblk *pblk, struct pblk_line *line)
> static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line)
> {
> 	struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> +	struct pblk_gc *gc = &pblk->gc;
> 
> 	spin_lock(&line->lock);
> 	WARN_ON(line->state != PBLK_LINESTATE_GC);
> @@ -1471,6 +1472,8 @@ static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line)
> 	pblk_line_free(pblk, line);
> 	spin_unlock(&line->lock);
> 
> +	atomic_dec(&gc->pipeline_gc);
> +
> 	spin_lock(&l_mg->free_lock);
> 	list_add_tail(&line->list, &l_mg->free_list);
> 	l_mg->nr_free_lines++;
> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> index f343f90..475a13e 100644
> --- a/drivers/lightnvm/pblk-gc.c
> +++ b/drivers/lightnvm/pblk-gc.c
> @@ -234,7 +234,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
> 	kfree(invalid_bitmap);
> 
> 	kref_put(&line->ref, pblk_line_put);
> -	atomic_dec(&gc->inflight_gc);
> +	atomic_dec(&gc->read_inflight_gc);
> 
> 	return;
> 
> @@ -249,7 +249,7 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
> 
> 	pblk_put_line_back(pblk, line);
> 	kref_put(&line->ref, pblk_line_put);
> -	atomic_dec(&gc->inflight_gc);
> +	atomic_dec(&gc->read_inflight_gc);
> 
> 	pr_err("pblk: Failed to GC line %d\n", line->id);
> }
> @@ -268,6 +268,7 @@ static int pblk_gc_line(struct pblk *pblk, struct pblk_line *line)
> 	line_ws->pblk = pblk;
> 	line_ws->line = line;
> 
> +	atomic_inc(&gc->pipeline_gc);
> 	INIT_WORK(&line_ws->ws, pblk_gc_line_prepare_ws);
> 	queue_work(gc->gc_reader_wq, &line_ws->ws);
> 
> @@ -333,6 +334,7 @@ static bool pblk_gc_should_run(struct pblk_gc *gc, struct pblk_rl *rl)
> void pblk_gc_free_full_lines(struct pblk *pblk)
> {
> 	struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> +	struct pblk_gc *gc = &pblk->gc;
> 	struct pblk_line *line;
> 
> 	do {
> @@ -353,6 +355,7 @@ void pblk_gc_free_full_lines(struct pblk *pblk)
> 		list_del(&line->list);
> 		spin_unlock(&l_mg->gc_lock);
> 
> +		atomic_inc(&gc->pipeline_gc);
> 		kref_put(&line->ref, pblk_line_put);
> 	} while (1);
> }
> @@ -370,12 +373,12 @@ static void pblk_gc_run(struct pblk *pblk)
> 	struct pblk_line *line;
> 	struct list_head *group_list;
> 	bool run_gc;
> -	int inflight_gc, gc_group = 0, prev_group = 0;
> +	int read_inflight_gc, gc_group = 0, prev_group = 0;
> 
> 	pblk_gc_free_full_lines(pblk);
> 
> 	run_gc = pblk_gc_should_run(&pblk->gc, &pblk->rl);
> -	if (!run_gc || (atomic_read(&gc->inflight_gc) >= PBLK_GC_L_QD))
> +	if (!run_gc || (atomic_read(&gc->read_inflight_gc) >= PBLK_GC_L_QD))
> 		return;
> 
> next_gc_group:
> @@ -402,14 +405,14 @@ static void pblk_gc_run(struct pblk *pblk)
> 		list_add_tail(&line->list, &gc->r_list);
> 		spin_unlock(&gc->r_lock);
> 
> -		inflight_gc = atomic_inc_return(&gc->inflight_gc);
> +		read_inflight_gc = atomic_inc_return(&gc->read_inflight_gc);
> 		pblk_gc_reader_kick(gc);
> 
> 		prev_group = 1;
> 
> 		/* No need to queue up more GC lines than we can handle */
> 		run_gc = pblk_gc_should_run(&pblk->gc, &pblk->rl);
> -		if (!run_gc || inflight_gc >= PBLK_GC_L_QD)
> +		if (!run_gc || read_inflight_gc >= PBLK_GC_L_QD)
> 			break;
> 	} while (1);
> 
> @@ -470,6 +473,7 @@ static int pblk_gc_writer_ts(void *data)
> static int pblk_gc_reader_ts(void *data)
> {
> 	struct pblk *pblk = data;
> +	struct pblk_gc *gc = &pblk->gc;
> 
> 	while (!kthread_should_stop()) {
> 		if (!pblk_gc_read(pblk))
> @@ -478,6 +482,18 @@ static int pblk_gc_reader_ts(void *data)
> 		io_schedule();
> 	}
> 
> +#ifdef CONFIG_NVM_DEBUG
> +	pr_info("pblk: flushing gc pipeline, %d lines left\n",
> +		atomic_read(&gc->pipeline_gc));
> +#endif
> +
> +	do {
> +		if (!atomic_read(&gc->pipeline_gc))
> +			break;
> +
> +		schedule();
> +	} while (1);
> +
> 	return 0;
> }
> 
> @@ -581,7 +597,8 @@ int pblk_gc_init(struct pblk *pblk)
> 	gc->gc_forced = 0;
> 	gc->gc_enabled = 1;
> 	gc->w_entries = 0;
> -	atomic_set(&gc->inflight_gc, 0);
> +	atomic_set(&gc->read_inflight_gc, 0);
> +	atomic_set(&gc->pipeline_gc, 0);
> 
> 	/* Workqueue that reads valid sectors from a line and submit them to the
> 	 * GC writer to be recycled.
> diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c
> index 95fb434..cd49e88 100644
> --- a/drivers/lightnvm/pblk-sysfs.c
> +++ b/drivers/lightnvm/pblk-sysfs.c
> @@ -253,7 +253,7 @@ static ssize_t pblk_sysfs_lines(struct pblk *pblk, char *page)
> 	sz += snprintf(page + sz, PAGE_SIZE - sz,
> 		"GC: full:%d, high:%d, mid:%d, low:%d, empty:%d, queue:%d\n",
> 			gc_full, gc_high, gc_mid, gc_low, gc_empty,
> -			atomic_read(&pblk->gc.inflight_gc));
> +			atomic_read(&pblk->gc.read_inflight_gc));
> 
> 	sz += snprintf(page + sz, PAGE_SIZE - sz,
> 		"data (%d) cur:%d, left:%d, vsc:%d, s:%d, map:%d/%d (%d)\n",
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index f76583b..03965da 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -238,7 +238,10 @@ struct pblk_gc {
> 	struct timer_list gc_timer;
> 
> 	struct semaphore gc_sem;
> -	atomic_t inflight_gc;
> +	atomic_t read_inflight_gc; /* Number of lines with inflight GC reads */
> +	atomic_t pipeline_gc;	   /* Number of lines in the GC pipeline -
> +				    * started reads to finished writes
> +				    */
> 	int w_entries;
> 
> 	struct list_head w_list;
> --
> 2.7.4

LGTM.

Reviewed-by: Javier González <javier@...xlabs.com>

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ