From: Rakesh Pandit <rakesh@tuxera.com>
To: "Javier González" <jg@lightnvm.io>
Cc: "Matias Bjørling" <mb@lightnvm.io>,
linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] lightnvm: pblk: reuse pblk_gc_should_kick
Date: Fri, 22 Sep 2017 12:42:14 +0300 [thread overview]
Message-ID: <20170922094213.GA31904@dhcp-216.srv.tuxera.com> (raw)
In-Reply-To: <BBF0A7B6-682D-45C7-A087-4072F5B101CE@lightnvm.io>
On Fri, Sep 22, 2017 at 10:40:57AM +0200, Javier Gonz�lez wrote:
> > On 21 Sep 2017, at 13.25, Rakesh Pandit wrote:
> >
> > This is a trivial change which reuses pblk_gc_should_kick instead of
> > repeating it again in pblk_rl_free_lines_inc.
> >
> > Signed-off-by: Rakesh Pandit <rakesh@tuxera.com>
> > ---
> > drivers/lightnvm/pblk-core.c | 1 +
> > drivers/lightnvm/pblk-rl.c | 9 ---------
> > 2 files changed, 1 insertion(+), 9 deletions(-)
> >
> > diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> > index 64a6a25..a230125 100644
> > --- a/drivers/lightnvm/pblk-core.c
> > +++ b/drivers/lightnvm/pblk-core.c
> > @@ -1478,6 +1478,7 @@ static void __pblk_line_put(struct pblk *pblk, struct pblk_line *line)
> > spin_unlock(&l_mg->free_lock);
> >
> > pblk_rl_free_lines_inc(&pblk->rl, line);
> > + pblk_gc_should_kick(pblk);
> > }
> >
> > static void pblk_line_put_ws(struct work_struct *work)
> > diff --git a/drivers/lightnvm/pblk-rl.c b/drivers/lightnvm/pblk-rl.c
> > index 596bdec..e7c162a 100644
> > --- a/drivers/lightnvm/pblk-rl.c
> > +++ b/drivers/lightnvm/pblk-rl.c
> > @@ -129,18 +129,9 @@ static int pblk_rl_update_rates(struct pblk_rl *rl, unsigned long max)
> >
> > void pblk_rl_free_lines_inc(struct pblk_rl *rl, struct pblk_line *line)
> > {
> > - struct pblk *pblk = container_of(rl, struct pblk, rl);
> > int blk_in_line = atomic_read(&line->blk_in_line);
> > - int ret;
> >
> > atomic_add(blk_in_line, &rl->free_blocks);
> > - /* Rates will not change that often - no need to lock update */
> > - ret = pblk_rl_update_rates(rl, rl->rb_budget);
> > -
> > - if (ret == (PBLK_RL_MID | PBLK_RL_LOW))
> > - pblk_gc_should_start(pblk);
> > - else
> > - pblk_gc_should_stop(pblk);
> > }
> >
> > void pblk_rl_free_lines_dec(struct pblk_rl *rl, struct pblk_line *line)
> > --
> > 2.5.0
>
> Looking at the patch, I can see that a more general cleanup can be done.
> What do you think about this? We can merge it in this patch if you are OK
> with it.
Makes more sense. Feel free to merge. You have my Signed-off-by tag.
>
> From 944936663c7eef7bc32243b89cc554e40ec436c5 Mon Sep 17 00:00:00 2001
> From: Rakesh Pandit <rakesh@tuxera.com>
> Date: Thu, 21 Sep 2017 14:25:40 +0300
> Subject: [PATCH] lightnvm: pblk: reuse pblk_gc_should_kick
>
> This is a trivial change which reuses pblk_gc_should_kick instead of
> repeating it again in pblk_rl_free_lines_inc.
>
> Signed-off-by: Rakesh Pandit <rakesh@tuxera.com>
> ---
> drivers/lightnvm/pblk-core.c | 2 --
> drivers/lightnvm/pblk-rl.c | 33 +++++++++------------------------
> drivers/lightnvm/pblk.h | 1 -
> 3 files changed, 9 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/lightnvm/pblk-core.c b/drivers/lightnvm/pblk-core.c
> index 64a6a255514e..c92113b0a2e7 100644
> --- a/drivers/lightnvm/pblk-core.c
> +++ b/drivers/lightnvm/pblk-core.c
> @@ -1633,8 +1633,6 @@ void pblk_line_close(struct pblk *pblk, struct pblk_line *line)
>
> spin_unlock(&line->lock);
> spin_unlock(&l_mg->gc_lock);
> -
> - pblk_gc_should_kick(pblk);
> }
>
> void pblk_line_close_meta(struct pblk *pblk, struct pblk_line *line)
> diff --git a/drivers/lightnvm/pblk-rl.c b/drivers/lightnvm/pblk-rl.c
> index 596bdec433c3..0896439a91b0 100644
> --- a/drivers/lightnvm/pblk-rl.c
> +++ b/drivers/lightnvm/pblk-rl.c
> @@ -96,9 +96,11 @@ unsigned long pblk_rl_nr_free_blks(struct pblk_rl *rl)
> *
> * Only the total number of free blocks is used to configure the rate limiter.
> */
> -static int pblk_rl_update_rates(struct pblk_rl *rl, unsigned long max)
> +static void pblk_rl_update_rates(struct pblk_rl *rl)
> {
> + struct pblk *pblk = container_of(rl, struct pblk, rl);
> unsigned long free_blocks = pblk_rl_nr_free_blks(rl);
> + int max = rl->rb_budget;
>
> if (free_blocks >= rl->high) {
> rl->rb_user_max = max;
> @@ -124,23 +126,18 @@ static int pblk_rl_update_rates(struct pblk_rl *rl, unsigned long max)
> rl->rb_state = PBLK_RL_LOW;
> }
>
> - return rl->rb_state;
> + if (rl->rb_state == (PBLK_RL_MID | PBLK_RL_LOW))
> + pblk_gc_should_start(pblk);
> + else
> + pblk_gc_should_stop(pblk);
> }
>
> void pblk_rl_free_lines_inc(struct pblk_rl *rl, struct pblk_line *line)
> {
> - struct pblk *pblk = container_of(rl, struct pblk, rl);
> int blk_in_line = atomic_read(&line->blk_in_line);
> - int ret;
>
> atomic_add(blk_in_line, &rl->free_blocks);
> - /* Rates will not change that often - no need to lock update */
> - ret = pblk_rl_update_rates(rl, rl->rb_budget);
> -
> - if (ret == (PBLK_RL_MID | PBLK_RL_LOW))
> - pblk_gc_should_start(pblk);
> - else
> - pblk_gc_should_stop(pblk);
> + pblk_rl_update_rates(rl);
> }
>
> void pblk_rl_free_lines_dec(struct pblk_rl *rl, struct pblk_line *line)
> @@ -148,19 +145,7 @@ void pblk_rl_free_lines_dec(struct pblk_rl *rl, struct pblk_line *line)
> int blk_in_line = atomic_read(&line->blk_in_line);
>
> atomic_sub(blk_in_line, &rl->free_blocks);
> -}
> -
> -void pblk_gc_should_kick(struct pblk *pblk)
> -{
> - struct pblk_rl *rl = &pblk->rl;
> - int ret;
> -
> - /* Rates will not change that often - no need to lock update */
> - ret = pblk_rl_update_rates(rl, rl->rb_budget);
> - if (ret == (PBLK_RL_MID | PBLK_RL_LOW))
> - pblk_gc_should_start(pblk);
> - else
> - pblk_gc_should_stop(pblk);
> + pblk_rl_update_rates(rl);
> }
>
> int pblk_rl_high_thrs(struct pblk_rl *rl)
> diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h
> index eaf539715d71..d4b7dee0ef50 100644
> --- a/drivers/lightnvm/pblk.h
> +++ b/drivers/lightnvm/pblk.h
> @@ -829,7 +829,6 @@ int pblk_gc_init(struct pblk *pblk);
> void pblk_gc_exit(struct pblk *pblk);
> void pblk_gc_should_start(struct pblk *pblk);
> void pblk_gc_should_stop(struct pblk *pblk);
> -void pblk_gc_should_kick(struct pblk *pblk);
> void pblk_gc_kick(struct pblk *pblk);
> void pblk_gc_sysfs_state_show(struct pblk *pblk, int *gc_enabled,
> int *gc_active);
> --
> 2.7.4
>
next prev parent reply other threads:[~2017-09-22 9:42 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-21 11:25 [PATCH 1/6] lightnvm: pblk: reuse pblk_gc_should_kick Rakesh Pandit
2017-09-22 8:40 ` Javier González
2017-09-22 9:42 ` Rakesh Pandit [this message]
2017-09-22 10:04 ` Javier González
2017-09-25 10:24 ` Matias Bjørling
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170922094213.GA31904@dhcp-216.srv.tuxera.com \
--to=rakesh@tuxera.com \
--cc=jg@lightnvm.io \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mb@lightnvm.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).