Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190130102604.14496-1-javier@javigon.com> <CANr-nt27moc9eLMgXkoXQGR7GA=uRpg2u+v7aJxzNB_AGbw5hQ@mail.gmail.com>
 <E4DF0FFC-5A45-405D-A942-DD8572CC1BD1@javigon.com> <CANr-nt0FWrzsCmv=X8oqCSOBYTO7nZi5gtKJN13setFQ8MFjRg@mail.gmail.com>
 <3E3CC5BE-585A-4F0A-8EC7-7FE5F855E888@javigon.com>
In-Reply-To: <3E3CC5BE-585A-4F0A-8EC7-7FE5F855E888@javigon.com>
From:   Hans Holmberg <hans@owltronix.com>
Date:   Mon, 4 Feb 2019 22:07:32 +0100
Message-ID: <CANr-nt3_xLgZ3fY60jNyYEVY828ZAdi6KTMcw7MKFKg+u6CUXA@mail.gmail.com>
Subject: Re: [PATCH V2] lightnvm: pblk: prevent stall due to wb threshold
To:     =?UTF-8?Q?Javier_Gonz=C3=A1lez?= <javier@javigon.com>
Cc:     =?UTF-8?Q?Matias_Bj=C3=B8rling?= <mb@lightnvm.io>,
        Hans Holmberg <hans.holmberg@cnexlabs.com>,
        linux-block@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Feb 4, 2019 at 9:14 AM Javier Gonz=C3=A1lez <javier@javigon.com> wr=
ote:
>
>
> > On 31 Jan 2019, at 21.10, Hans Holmberg <hans@owltronix.com> wrote:
> >
> > On Thu, Jan 31, 2019 at 5:33 PM Javier Gonz=C3=A1lez <javier@javigon.co=
m> wrote:
> >>> On 31 Jan 2019, at 11.41, Hans Holmberg <hans@owltronix.com> wrote:
> >>>
> >>> Hi Javier!
> >>>
> >>> How did you test this? I'm trying to add a test case to our testing f=
ramework.
> >>>
> >>> This is what i ran in qemu, and I got a hang (with this version of th=
e patch)
> >>>
> >>> nvme lnvm create -d nvme0n1 -t pblk -n pblk0 -f -b 0 -e 0
> >>
> >> I run several low configurations without problem. Can you share the qe=
mu configuration and version?
> >
> > Of course!
> >
> > qemu remote: https://github.com/CNEX-Labs/qemu-nvme.git
> > branch: master (cb200e3ccf9c1ff21f6275c6cb68b2801135a640)
> >
> > My geometry:
> > parallel units: 8, secs per chk: 768, meta size 16, ws min 12, ws opt
> > 24, cunits=3D0
>
> You are right, lmw_cunits=3D0 gives the problem.
>
> What about the following? If it goes through cijoe, I'll send a V3.

Please do send a V3 if it passes your testing.

Thanks!
Hans

>
> 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 =3D 1 << (get_count_order(threshold + NVM_MAX=
_VLBA));
> +       unsigned int max_sz =3D 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 tha=
t the
> +        * threshold will be respected
> +        */
> +       max_io =3D (1 << max((int)(get_count_order(max_sz)),
> +                               (int)(get_count_order(NVM_MAX_VLBA << 1))=
));
> +       if ((threshold + NVM_MAX_VLBA) >=3D max_io)
> +               max_io <<=3D 1;
> +
> +       return max_io;
>  }
>
>  /*
> @@ -67,12 +80,12 @@ int pblk_rb_init(struct pblk_rb *rb, unsigned int siz=
e, unsigned int threshold,
>         unsigned int alloc_order, order, iter;
>         unsigned int nr_entries;
>
> -       nr_entries =3D pblk_rb_calculate_size(size);
> +       nr_entries =3D pblk_rb_calculate_size(size, threshold);
>         entries =3D vzalloc(array_size(nr_entries, sizeof(struct pblk_rb_=
entry)));
>         if (!entries)
>                 return -ENOMEM;
>
> -       power_size =3D get_count_order(size);
> +       power_size =3D get_count_order(nr_entries);
>         power_seg_sz =3D get_count_order(seg_size);
>
>         down_write(&pblk_rb_lock);
> @@ -149,7 +162,7 @@ int pblk_rb_init(struct pblk_rb *rb, unsigned int siz=
e, unsigned int threshold,
>          * Initialize rate-limiter, which controls access to the write bu=
ffer
>          * 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 =3D container_of(rl, struct pblk, rl);
>         struct nvm_tgt_dev *dev =3D 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 =3D (lm->smeta_sec + lm->emeta_sec[0]) * l_mg->nr_free_l=
ines;
>         blk_meta =3D 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 =3D budget;
>         rl->rb_user_max =3D budget;
> -       rl->rb_max_io =3D budget >> 1;
> +       rl->rb_max_io =3D threshold ? (budget - threshold) : (budget - 1)=
;
>         rl->rb_gc_max =3D 0;
>         rl->rb_state =3D 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.17.1
>