* [PATCH v2 0/2] Fix corner case in bad block table handling. @ 2021-04-06 1:46 Yoshio Furuyama 2021-04-06 1:47 ` [PATCH v2 1/2] " Yoshio Furuyama 2021-04-06 1:47 ` [PATCH v2 2/2] Fix the issue for clearing status process Yoshio Furuyama 0 siblings, 2 replies; 8+ messages in thread From: Yoshio Furuyama @ 2021-04-06 1:46 UTC (permalink / raw) To: miquel.raynal, richard, vigneshr; +Cc: linux-mtd, linux-kernel Doyle, Patrick (1): Fix corner case in bad block table handling. Yoshio Furuyama (1): Fix the issue for clearing status process drivers/mtd/nand/bbt.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] Fix corner case in bad block table handling. 2021-04-06 1:46 [PATCH v2 0/2] Fix corner case in bad block table handling Yoshio Furuyama @ 2021-04-06 1:47 ` Yoshio Furuyama 2021-05-10 10:52 ` Miquel Raynal 2022-01-11 15:33 ` Frieder Schrempf 2021-04-06 1:47 ` [PATCH v2 2/2] Fix the issue for clearing status process Yoshio Furuyama 1 sibling, 2 replies; 8+ messages in thread From: Yoshio Furuyama @ 2021-04-06 1:47 UTC (permalink / raw) To: miquel.raynal, richard, vigneshr; +Cc: linux-mtd, linux-kernel From: "Doyle, Patrick" <pdoyle@irobot.com> In the unlikely event that both blocks 10 and 11 are marked as bad (on a 32 bit machine), then the process of marking block 10 as bad stomps on cached entry for block 11. There are (of course) other examples. Signed-off-by: Patrick Doyle <pdoyle@irobot.com> Reviewed-by: Richard Weinberger <richard@nod.at> --- drivers/mtd/nand/bbt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/nand/bbt.c b/drivers/mtd/nand/bbt.c index 044adf913854..64af6898131d 100644 --- a/drivers/mtd/nand/bbt.c +++ b/drivers/mtd/nand/bbt.c @@ -123,7 +123,7 @@ int nanddev_bbt_set_block_status(struct nand_device *nand, unsigned int entry, unsigned int rbits = bits_per_block + offs - BITS_PER_LONG; pos[1] &= ~GENMASK(rbits - 1, 0); - pos[1] |= val >> rbits; + pos[1] |= val >> (bits_per_block - rbits); } return 0; -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] Fix corner case in bad block table handling. 2021-04-06 1:47 ` [PATCH v2 1/2] " Yoshio Furuyama @ 2021-05-10 10:52 ` Miquel Raynal 2022-01-11 15:33 ` Frieder Schrempf 1 sibling, 0 replies; 8+ messages in thread From: Miquel Raynal @ 2021-05-10 10:52 UTC (permalink / raw) To: Yoshio Furuyama, miquel.raynal, richard, vigneshr; +Cc: linux-mtd, linux-kernel On Tue, 2021-04-06 at 01:47:08 UTC, Yoshio Furuyama wrote: > From: "Doyle, Patrick" <pdoyle@irobot.com> > > In the unlikely event that both blocks 10 and 11 are marked as bad (on a > 32 bit machine), then the process of marking block 10 as bad stomps on > cached entry for block 11. There are (of course) other examples. > > Signed-off-by: Patrick Doyle <pdoyle@irobot.com> > Reviewed-by: Richard Weinberger <richard@nod.at> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks. Miquel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] Fix corner case in bad block table handling. 2021-04-06 1:47 ` [PATCH v2 1/2] " Yoshio Furuyama 2021-05-10 10:52 ` Miquel Raynal @ 2022-01-11 15:33 ` Frieder Schrempf 2022-01-24 14:11 ` Frieder Schrempf 1 sibling, 1 reply; 8+ messages in thread From: Frieder Schrempf @ 2022-01-11 15:33 UTC (permalink / raw) To: stable Cc: linux-mtd, linux-kernel, vigneshr, miquel.raynal, Yoshio Furuyama, richard, Stoll Eberhard Hi stable maintainers, On 06.04.21 03:47, Yoshio Furuyama wrote: > From: "Doyle, Patrick" <pdoyle@irobot.com> > > In the unlikely event that both blocks 10 and 11 are marked as bad (on a > 32 bit machine), then the process of marking block 10 as bad stomps on > cached entry for block 11. There are (of course) other examples. > > Signed-off-by: Patrick Doyle <pdoyle@irobot.com> > Reviewed-by: Richard Weinberger <richard@nod.at> We have systems on which this patch fixes real failures. Could you please add the upstream patch fd0d8d85f723 ("mtd: nand: bbt: Fix corner case in bad block table handling") to the stable queues for 4.19, 5.4, 5.10? Thanks! Cc: stable@vger.kernel.org Fixes: 9c3736a3de21 ("mtd: nand: Add core infrastructure to deal with NAND devices") > --- > drivers/mtd/nand/bbt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/bbt.c b/drivers/mtd/nand/bbt.c > index 044adf913854..64af6898131d 100644 > --- a/drivers/mtd/nand/bbt.c > +++ b/drivers/mtd/nand/bbt.c > @@ -123,7 +123,7 @@ int nanddev_bbt_set_block_status(struct nand_device *nand, unsigned int entry, > unsigned int rbits = bits_per_block + offs - BITS_PER_LONG; > > pos[1] &= ~GENMASK(rbits - 1, 0); > - pos[1] |= val >> rbits; > + pos[1] |= val >> (bits_per_block - rbits); > } > > return 0; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] Fix corner case in bad block table handling. 2022-01-11 15:33 ` Frieder Schrempf @ 2022-01-24 14:11 ` Frieder Schrempf 2022-01-24 14:29 ` Greg Kroah-Hartman 0 siblings, 1 reply; 8+ messages in thread From: Frieder Schrempf @ 2022-01-24 14:11 UTC (permalink / raw) To: stable, Greg Kroah-Hartman, sashal Cc: linux-mtd, linux-kernel, vigneshr, miquel.raynal, Yoshio Furuyama, richard, Stoll Eberhard Hi Greg, Sasha, just a gentle ping for the backport request below. Thanks! Am 11.01.22 um 16:33 schrieb Frieder Schrempf: > Hi stable maintainers, > > On 06.04.21 03:47, Yoshio Furuyama wrote: >> From: "Doyle, Patrick" <pdoyle@irobot.com> >> >> In the unlikely event that both blocks 10 and 11 are marked as bad (on a >> 32 bit machine), then the process of marking block 10 as bad stomps on >> cached entry for block 11. There are (of course) other examples. >> >> Signed-off-by: Patrick Doyle <pdoyle@irobot.com> >> Reviewed-by: Richard Weinberger <richard@nod.at> > > We have systems on which this patch fixes real failures. Could you > please add the upstream patch fd0d8d85f723 ("mtd: nand: bbt: Fix corner > case in bad block table handling") to the stable queues for 4.19, 5.4, 5.10? > > Thanks! > > Cc: stable@vger.kernel.org > Fixes: 9c3736a3de21 ("mtd: nand: Add core infrastructure to deal with > NAND devices") > >> --- >> drivers/mtd/nand/bbt.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/nand/bbt.c b/drivers/mtd/nand/bbt.c >> index 044adf913854..64af6898131d 100644 >> --- a/drivers/mtd/nand/bbt.c >> +++ b/drivers/mtd/nand/bbt.c >> @@ -123,7 +123,7 @@ int nanddev_bbt_set_block_status(struct nand_device *nand, unsigned int entry, >> unsigned int rbits = bits_per_block + offs - BITS_PER_LONG; >> >> pos[1] &= ~GENMASK(rbits - 1, 0); >> - pos[1] |= val >> rbits; >> + pos[1] |= val >> (bits_per_block - rbits); >> } >> >> return 0; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] Fix corner case in bad block table handling. 2022-01-24 14:11 ` Frieder Schrempf @ 2022-01-24 14:29 ` Greg Kroah-Hartman 0 siblings, 0 replies; 8+ messages in thread From: Greg Kroah-Hartman @ 2022-01-24 14:29 UTC (permalink / raw) To: Frieder Schrempf Cc: stable, sashal, linux-mtd, linux-kernel, vigneshr, miquel.raynal, Yoshio Furuyama, richard, Stoll Eberhard On Mon, Jan 24, 2022 at 03:11:14PM +0100, Frieder Schrempf wrote: > Hi Greg, Sasha, > > just a gentle ping for the backport request below. > > Thanks! > > Am 11.01.22 um 16:33 schrieb Frieder Schrempf: > > Hi stable maintainers, > > > > On 06.04.21 03:47, Yoshio Furuyama wrote: > >> From: "Doyle, Patrick" <pdoyle@irobot.com> > >> > >> In the unlikely event that both blocks 10 and 11 are marked as bad (on a > >> 32 bit machine), then the process of marking block 10 as bad stomps on > >> cached entry for block 11. There are (of course) other examples. > >> > >> Signed-off-by: Patrick Doyle <pdoyle@irobot.com> > >> Reviewed-by: Richard Weinberger <richard@nod.at> > > > > We have systems on which this patch fixes real failures. Could you > > please add the upstream patch fd0d8d85f723 ("mtd: nand: bbt: Fix corner > > case in bad block table handling") to the stable queues for 4.19, 5.4, 5.10? > > > > Thanks! > > > > Cc: stable@vger.kernel.org > > Fixes: 9c3736a3de21 ("mtd: nand: Add core infrastructure to deal with > > NAND devices") Odd, I did not see this anywhere in my inbox. Now queued up, thanks. greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] Fix the issue for clearing status process 2021-04-06 1:46 [PATCH v2 0/2] Fix corner case in bad block table handling Yoshio Furuyama 2021-04-06 1:47 ` [PATCH v2 1/2] " Yoshio Furuyama @ 2021-04-06 1:47 ` Yoshio Furuyama 2021-04-07 9:04 ` Miquel Raynal 1 sibling, 1 reply; 8+ messages in thread From: Yoshio Furuyama @ 2021-04-06 1:47 UTC (permalink / raw) To: miquel.raynal, richard, vigneshr; +Cc: linux-mtd, linux-kernel In the unlikely event of bad block, it should update its block status to BBT, In this case, there are 2 kind of issue for handling a) Mark bad block status to BBT: It was fixed by Patric's patch b) Clear status to BBT: I posted patch for this issue Patch: Issue of handing BBT (Bad Block Table) for some particular blocks (Ex:10, 11) Updating status is, first clear status, second set bad block status. Patrick's patch is only fixed the issue for setting status process, so this patch fix the clearing status process. Signed-off-by: Yoshio Furuyama <ytc-mb-yfuruyama7@kioxia.com> --- drivers/mtd/nand/bbt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/nand/bbt.c b/drivers/mtd/nand/bbt.c index 64af6898131d..0780896eaafe 100644 --- a/drivers/mtd/nand/bbt.c +++ b/drivers/mtd/nand/bbt.c @@ -112,11 +112,13 @@ int nanddev_bbt_set_block_status(struct nand_device *nand, unsigned int entry, ((entry * bits_per_block) / BITS_PER_LONG); unsigned int offs = (entry * bits_per_block) % BITS_PER_LONG; unsigned long val = status & GENMASK(bits_per_block - 1, 0); + unsigned long shift = ((bits_per_block + offs <= BITS_PER_LONG) ? + (offs + bits_per_block - 1) : (BITS_PER_LONG - 1)); if (entry >= nanddev_neraseblocks(nand)) return -ERANGE; - pos[0] &= ~GENMASK(offs + bits_per_block - 1, offs); + pos[0] &= ~GENMASK(shift, offs); pos[0] |= val << offs; if (bits_per_block + offs > BITS_PER_LONG) { -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] Fix the issue for clearing status process 2021-04-06 1:47 ` [PATCH v2 2/2] Fix the issue for clearing status process Yoshio Furuyama @ 2021-04-07 9:04 ` Miquel Raynal 0 siblings, 0 replies; 8+ messages in thread From: Miquel Raynal @ 2021-04-07 9:04 UTC (permalink / raw) To: Yoshio Furuyama; +Cc: richard, vigneshr, linux-mtd, linux-kernel Hi Yoshio, Yoshio Furuyama <ytc-mb-yfuruyama7@kioxia.com> wrote on Tue, 6 Apr 2021 10:47:26 +0900: Could you add "mtd: nand: bbt:" as prefix for the title (same for the other patch, even though you're not the original author). > In the unlikely event of bad block, > it should update its block status to BBT, > In this case, there are 2 kind of issue for handling > a) Mark bad block status to BBT: It was fixed by Patric's patch > b) Clear status to BBT: I posted patch for this issue > > Patch: > Issue of handing BBT (Bad Block Table) for > some particular blocks (Ex:10, 11) > Updating status is, first clear status, second set bad block status. > Patrick's patch is only fixed the issue for setting status process, > so this patch fix the clearing status process. This commit message is not clearly describing the situation, could you please reword it? > > Signed-off-by: Yoshio Furuyama <ytc-mb-yfuruyama7@kioxia.com> > --- > drivers/mtd/nand/bbt.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/bbt.c b/drivers/mtd/nand/bbt.c > index 64af6898131d..0780896eaafe 100644 > --- a/drivers/mtd/nand/bbt.c > +++ b/drivers/mtd/nand/bbt.c > @@ -112,11 +112,13 @@ int nanddev_bbt_set_block_status(struct nand_device *nand, unsigned int entry, > ((entry * bits_per_block) / BITS_PER_LONG); > unsigned int offs = (entry * bits_per_block) % BITS_PER_LONG; > unsigned long val = status & GENMASK(bits_per_block - 1, 0); > + unsigned long shift = ((bits_per_block + offs <= BITS_PER_LONG) ? > + (offs + bits_per_block - 1) : (BITS_PER_LONG - 1)); Given the fact that we do arithmetic operations (&, |) on an unsigned long value I don't think the operation tampers with the next entry in the pos array. I'm fine fixing it but I don't think this implementation works. It is fine if offs is 29 or 30 but not if it is 31 (assuming 32-bits arithmetic, it's the same for the 64-bit case). > > if (entry >= nanddev_neraseblocks(nand)) > return -ERANGE; > > - pos[0] &= ~GENMASK(offs + bits_per_block - 1, offs); Would something like the following work? pos[0] &= ~GENMASK(MIN(offs + bits_per_block - 1, BITS_PER_LONG - 1), offs) Again, I am not convinced it is worth darkening the logic unless I am not understanding it correctly. > + pos[0] &= ~GENMASK(shift, offs); > pos[0] |= val << offs; > > if (bits_per_block + offs > BITS_PER_LONG) { Thanks, Miquèl ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-01-24 14:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-06 1:46 [PATCH v2 0/2] Fix corner case in bad block table handling Yoshio Furuyama 2021-04-06 1:47 ` [PATCH v2 1/2] " Yoshio Furuyama 2021-05-10 10:52 ` Miquel Raynal 2022-01-11 15:33 ` Frieder Schrempf 2022-01-24 14:11 ` Frieder Schrempf 2022-01-24 14:29 ` Greg Kroah-Hartman 2021-04-06 1:47 ` [PATCH v2 2/2] Fix the issue for clearing status process Yoshio Furuyama 2021-04-07 9:04 ` Miquel Raynal
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).