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
From: "Doyle, Patrick" <[email protected]>
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 <[email protected]>
Reviewed-by: Richard Weinberger <[email protected]>
---
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
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 <[email protected]>
---
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
Hi Yoshio,
Yoshio Furuyama <[email protected]> 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 <[email protected]>
> ---
> 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
On Tue, 2021-04-06 at 01:47:08 UTC, Yoshio Furuyama wrote:
> From: "Doyle, Patrick" <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Richard Weinberger <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
Hi stable maintainers,
On 06.04.21 03:47, Yoshio Furuyama wrote:
> From: "Doyle, Patrick" <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Richard Weinberger <[email protected]>
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: [email protected]
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;
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" <[email protected]>
>>
>> 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 <[email protected]>
>> Reviewed-by: Richard Weinberger <[email protected]>
>
> 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: [email protected]
> 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;
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" <[email protected]>
> >>
> >> 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 <[email protected]>
> >> Reviewed-by: Richard Weinberger <[email protected]>
> >
> > 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: [email protected]
> > 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