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 2/6] lightnvm: pblk: reduce arguments in __pblk_rb_update_l2p
Date: Mon, 2 Oct 2017 20:23:38 +0300 [thread overview]
Message-ID: <20171002172338.GC6008@hercules.tuxera.com> (raw)
In-Reply-To: <5330486D-DD91-4980-B002-6BD9255835A4@lightnvm.io>
On Mon, Oct 02, 2017 at 05:16:57PM +0200, Javier González wrote:
> > On 2 Oct 2017, at 13.32, Javier González <jg@lightnvm.io> wrote:
> >
> >> On 1 Oct 2017, at 15.23, Rakesh Pandit <rakesh@tuxera.com> wrote:
[..]
> >> -static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
> >> - unsigned int to_update)
> >> +static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int to_update)
> >> {
> >> + unsigned int l2p_update = rb->l2p_update;
> >> struct pblk *pblk = container_of(rb, struct pblk, rwb);
> >> struct pblk_line *line;
> >> struct pblk_rb_entry *entry;
> >> @@ -213,7 +213,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
> >> int flags;
> >>
> >> for (i = 0; i < to_update; i++) {
> >> - entry = &rb->entries[*l2p_upd];
> >> + entry = &rb->entries[l2p_update];
> >> w_ctx = &entry->w_ctx;
> >>
> >> flags = READ_ONCE(entry->w_ctx.flags);
> >> @@ -230,7 +230,7 @@ static int __pblk_rb_update_l2p(struct pblk_rb *rb, unsigned int *l2p_upd,
> >> line = &pblk->lines[pblk_tgt_ppa_to_line(w_ctx->ppa)];
> >> kref_put(&line->ref, pblk_line_put);
> >> clean_wctx(w_ctx);
> >> - *l2p_upd = (*l2p_upd + 1) & (rb->nr_entries - 1);
> >> + rb->l2p_update = (l2p_update + 1) & (rb->nr_entries - 1);
>
> This is wrong. It should be rb->l2p_update when doing +1, otherwise you
> are not using the updated l2p_update value. The result is the pipeline
> stalling as the l2p pointer is left behind.
>
> I'll fix it when picking it up.
>
> Please test the patches before submitting.
Thanks for fixing it. I would be more careful. And will make sure
everything goes through testing before.
Regards,
prev parent reply other threads:[~2017-10-02 17:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-01 13:23 Rakesh Pandit
2017-10-02 11:32 ` Javier González
2017-10-02 15:16 ` Javier González
2017-10-02 17:23 ` Rakesh Pandit [this message]
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=20171002172338.GC6008@hercules.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
Powered by JetHome