From: Hans Holmberg <hans.ml.holmberg@owltronix.com>
To: sboisvert@gydle.com
Cc: Matias Bjorling <mb@lightnvm.io>,
Javier Gonzalez <javier@cnexlabs.com>,
linux-block@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Hans Holmberg <hans.holmberg@cnexlabs.com>
Subject: Re: [PATCH v2 1/7] lightnvm: pblk: fix resubmission of overwritten write err lbas
Date: Tue, 6 Nov 2018 12:52:25 +0100 [thread overview]
Message-ID: <CANr-nt1Juq4OX3H+c2B4F4xRYO=HZxu_Sx+_d+0eOi4Y5gqGTA@mail.gmail.com> (raw)
In-Reply-To: <de7235bc-e458-381d-442e-11b42dd20dc9@gydle.com>
On Mon, Nov 5, 2018 at 4:12 PM Sebastien Boisvert <sboisvert@gydle.com> wrote:
>
>
>
> On 2018-11-05 7:26 a.m., Hans Holmberg wrote:
> > From: Hans Holmberg <hans.holmberg@cnexlabs.com>
> >
> > Make sure we only look up valid lba addresses on the resubmission path.
> >
> > If an lba is invalidated in the write buffer, that sector will be
> > submitted to disk(as it is already mapped to a ppa), and that write
>
> submitted to disk(as it is already mapped to a ppa), and that write
> ^
> add a space
>
> > might fail, resulting in a crash when trying to look up the lba in the
> > mapping table (as the lba is marked as invalid).
> >
> > Signed-off-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> > ---
> > drivers/lightnvm/pblk-write.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/lightnvm/pblk-write.c b/drivers/lightnvm/pblk-write.c
> > index fa8726493b39..3ddd16f47106 100644
> > --- a/drivers/lightnvm/pblk-write.c
> > +++ b/drivers/lightnvm/pblk-write.c
> > @@ -148,9 +148,11 @@ static void pblk_prepare_resubmit(struct pblk *pblk, unsigned int sentry,
> > w_ctx = &entry->w_ctx;
> >
> > /* Check if the lba has been overwritten */
> > - ppa_l2p = pblk_trans_map_get(pblk, w_ctx->lba);
> > - if (!pblk_ppa_comp(ppa_l2p, entry->cacheline))
> > - w_ctx->lba = ADDR_EMPTY;
> > + if (w_ctx->lba != ADDR_EMPTY) {
> > + ppa_l2p = pblk_trans_map_get(pblk, w_ctx->lba);
> > + if (!pblk_ppa_comp(ppa_l2p, entry->cacheline))
> > + w_ctx->lba = ADDR_EMPTY;
> > + }
>
> Was w_ctx->lba set to ADDR_EMPTY in the same kernel I/O thread ?
>
> I wonder if w_ctx->lba could become ADDR_EMPTY after
>
> if (w_ctx->lba != ADDR_EMPTY) {
>
> but before
>
> ppa_l2p = pblk_trans_map_get(pblk, w_ctx->lba);
It can't, the lba adress can only be changed on the write error
handling path, and we'll have to resubmit the write before that can
happen.
Thanks for the review.
/ Hans
>
>
> >
> > /* Mark up the entry as submittable again */
> > flags = READ_ONCE(w_ctx->flags);
> >
next prev parent reply other threads:[~2018-11-06 11:52 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-05 12:26 [PATCH v2 0/7] PBLK Bugfixes and cleanups Hans Holmberg
2018-11-05 12:26 ` [PATCH v2 1/7] lightnvm: pblk: fix resubmission of overwritten write err lbas Hans Holmberg
2018-11-05 15:12 ` Sebastien Boisvert
2018-11-06 11:52 ` Hans Holmberg [this message]
2018-11-05 12:26 ` [PATCH v2 2/7] lightnvm: pblk: account for write error sectors in emeta Hans Holmberg
2018-11-05 12:26 ` [PATCH v2 3/7] lightnvm: pblk: stop writes gracefully when running out of lines Hans Holmberg
2018-11-05 12:26 ` [PATCH v2 4/7] lightnvm: pblk: set conservative threshold for user writes Hans Holmberg
2018-11-05 21:15 ` kbuild test robot
2018-11-05 23:09 ` kbuild test robot
2018-11-05 12:26 ` [PATCH v2 5/7] lightnvm: pblk: remove unused macro Hans Holmberg
2018-11-05 12:26 ` [PATCH v2 6/7] lightnvm: pblk: fix pblk_lines_init error handling path Hans Holmberg
2018-11-05 12:26 ` [PATCH v2 7/7] lightnvm: pblk: remove dead code in pblk_recov_l2p Hans Holmberg
2018-11-05 15:31 ` Sebastien Boisvert
2018-11-06 9:18 ` [PATCH v2 0/7] PBLK Bugfixes and cleanups Javier Gonzalez
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='CANr-nt1Juq4OX3H+c2B4F4xRYO=HZxu_Sx+_d+0eOi4Y5gqGTA@mail.gmail.com' \
--to=hans.ml.holmberg@owltronix.com \
--cc=hans.holmberg@cnexlabs.com \
--cc=javier@cnexlabs.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mb@lightnvm.io \
--cc=sboisvert@gydle.com \
/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).