Re: [PATCH 02/13] lightnvm: pblk: Gracefully handle GC data malloc fail

Javier González
2019-02-28T17:09:02Z

[-- Attachment #1: Type: text/plain, Size: 2776 bytes --]

> On 27 Feb 2019, at 12.14, Igor Konopko <igor.j.konopko@intel.com> wrote:
>
> Currently when we fail on gc rq data allocation
> we simply skip the data which we wanted to move
> and finally move the line to free state and lose
> that data due to that. This patch move the data
> allocation to some earlier phase of GC, where we
> can still fail gracefully by moving line back
> to closed state.
>
> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com>
> ---
> drivers/lightnvm/pblk-gc.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-gc.c b/drivers/lightnvm/pblk-gc.c
> index 3feadfd9418d..31fc1339faa8 100644
> --- a/drivers/lightnvm/pblk-gc.c
> +++ b/drivers/lightnvm/pblk-gc.c
> @@ -84,8 +84,6 @@ static void pblk_gc_line_ws(struct work_struct *work)
> struct pblk_line_ws *gc_rq_ws = container_of(work,
> struct pblk_line_ws, ws);
> struct pblk *pblk = gc_rq_ws->pblk;
> - struct nvm_tgt_dev *dev = pblk->dev;
> - struct nvm_geo *geo = &dev->geo;
> struct pblk_gc *gc = &pblk->gc;
> struct pblk_line *line = gc_rq_ws->line;
> struct pblk_gc_rq *gc_rq = gc_rq_ws->priv;
> @@ -93,13 +91,6 @@ static void pblk_gc_line_ws(struct work_struct *work)
>
> up(&gc->gc_sem);
>
> - gc_rq->data = vmalloc(array_size(gc_rq->nr_secs, geo->csecs));
> - if (!gc_rq->data) {
> - pblk_err(pblk, "could not GC line:%d (%d/%d)\n",
> - line->id, *line->vsc, gc_rq->nr_secs);
> - goto out;
> - }
> -
> /* Read from GC victim block */
> ret = pblk_submit_read_gc(pblk, gc_rq);
> if (ret) {
> @@ -189,6 +180,8 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
> struct pblk_line *line = line_ws->line;
> struct pblk_line_mgmt *l_mg = &pblk->l_mg;
> struct pblk_line_meta *lm = &pblk->lm;
> + struct nvm_tgt_dev *dev = pblk->dev;
> + struct nvm_geo *geo = &dev->geo;
> struct pblk_gc *gc = &pblk->gc;
> struct pblk_line_ws *gc_rq_ws;
> struct pblk_gc_rq *gc_rq;
> @@ -247,9 +240,13 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
> gc_rq->nr_secs = nr_secs;
> gc_rq->line = line;
>
> + gc_rq->data = vmalloc(gc_rq->nr_secs * geo->csecs);
> + if (!gc_rq->data)
> + goto fail_free_gc_rq;
> +
> gc_rq_ws = kmalloc(sizeof(struct pblk_line_ws), GFP_KERNEL);
> if (!gc_rq_ws)
> - goto fail_free_gc_rq;
> + goto fail_free_gc_data;
>
> gc_rq_ws->pblk = pblk;
> gc_rq_ws->line = line;
> @@ -281,6 +278,8 @@ static void pblk_gc_line_prepare_ws(struct work_struct *work)
>
> return;
>
> +fail_free_gc_data:
> + vfree(gc_rq->data);
> fail_free_gc_rq:
> kfree(gc_rq);
> fail_free_lba_list:
> --
> 2.17.1

Looks good to me.

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

[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]