diff mbox

[1/6] UBI: Fastmap: Care about the protection queue

Message ID 1416835236-25185-2-git-send-email-richard@nod.at
State Accepted
Headers show

Commit Message

Richard Weinberger Nov. 24, 2014, 1:20 p.m. UTC
Fastmap can miss a PEB if it is in the protection queue
and not jet in the used tree.
Treat every protected PEB as used.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 drivers/mtd/ubi/fastmap.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Artem Bityutskiy Nov. 27, 2014, 2:54 p.m. UTC | #1
On Mon, 2014-11-24 at 14:20 +0100, Richard Weinberger wrote:
> Fastmap can miss a PEB if it is in the protection queue
> and not jet in the used tree.
> Treat every protected PEB as used.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>

Picked this one and pushed, thanks!
Ezequiel Garcia Jan. 9, 2015, 9:23 p.m. UTC | #2
On 11/24/2014 10:20 AM, Richard Weinberger wrote:
> Fastmap can miss a PEB if it is in the protection queue
> and not jet in the used tree.

s/jet/yet

> Treat every protected PEB as used.
> 

I'm wondering if we can have a detailed description of the issues this
commit fixes (if any?). In other words, what's the result of *not*
having the patch?

> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  drivers/mtd/ubi/fastmap.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
> index b56672b..db3defd 100644
> --- a/drivers/mtd/ubi/fastmap.c
> +++ b/drivers/mtd/ubi/fastmap.c
> @@ -1196,6 +1196,19 @@ static int ubi_write_fastmap(struct ubi_device *ubi,
>  		fm_pos += sizeof(*fec);
>  		ubi_assert(fm_pos <= ubi->fm_size);
>  	}
> +
> +	for (i = 0; i < UBI_PROT_QUEUE_LEN; i++) {
> +		list_for_each_entry(wl_e, &ubi->pq[i], u.list) {
> +			fec = (struct ubi_fm_ec *)(fm_raw + fm_pos);
> +
> +			fec->pnum = cpu_to_be32(wl_e->pnum);
> +			fec->ec = cpu_to_be32(wl_e->ec);
> +
> +			used_peb_count++;
> +			fm_pos += sizeof(*fec);
> +			ubi_assert(fm_pos <= ubi->fm_size);
> +		}
> +	}
>  	fmh->used_peb_count = cpu_to_be32(used_peb_count);
>  
>  	for (node = rb_first(&ubi->scrub); node; node = rb_next(node)) {
>
Richard Weinberger Jan. 9, 2015, 9:31 p.m. UTC | #3
Am 09.01.2015 um 22:23 schrieb Ezequiel Garcia:
> On 11/24/2014 10:20 AM, Richard Weinberger wrote:
>> Fastmap can miss a PEB if it is in the protection queue
>> and not jet in the used tree.
> 
> s/jet/yet
> 
>> Treat every protected PEB as used.
>>
> 
> I'm wondering if we can have a detailed description of the issues this
> commit fixes (if any?). In other words, what's the result of *not*
> having the patch?

Without this patch fastmap can miss one PEB.
As consequence of this during attach the fastmap self-check
will detect this inconsistency and will fall back to a full scan.
If you try to attach using a custom fastmap implementation (found in
bootloaders) which do often not have sophisticated self-checks you'll
face a serious data corruption or if you're lucky UBI will crash.

Now clearer? :-)

Thanks,
//richard
Ezequiel Garcia Jan. 9, 2015, 9:34 p.m. UTC | #4
On 01/09/2015 06:31 PM, Richard Weinberger wrote:
> Am 09.01.2015 um 22:23 schrieb Ezequiel Garcia:
>> On 11/24/2014 10:20 AM, Richard Weinberger wrote:
>>> Fastmap can miss a PEB if it is in the protection queue
>>> and not jet in the used tree.
>>
>> s/jet/yet
>>
>>> Treat every protected PEB as used.
>>>
>>
>> I'm wondering if we can have a detailed description of the issues this
>> commit fixes (if any?). In other words, what's the result of *not*
>> having the patch?
> 
> Without this patch fastmap can miss one PEB.
> As consequence of this during attach the fastmap self-check
> will detect this inconsistency and will fall back to a full scan.
> If you try to attach using a custom fastmap implementation (found in
> bootloaders) which do often not have sophisticated self-checks you'll
> face a serious data corruption or if you're lucky UBI will crash.
> 
> Now clearer? :-)
> 

Much better!

Thanks a lot,
diff mbox

Patch

diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index b56672b..db3defd 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -1196,6 +1196,19 @@  static int ubi_write_fastmap(struct ubi_device *ubi,
 		fm_pos += sizeof(*fec);
 		ubi_assert(fm_pos <= ubi->fm_size);
 	}
+
+	for (i = 0; i < UBI_PROT_QUEUE_LEN; i++) {
+		list_for_each_entry(wl_e, &ubi->pq[i], u.list) {
+			fec = (struct ubi_fm_ec *)(fm_raw + fm_pos);
+
+			fec->pnum = cpu_to_be32(wl_e->pnum);
+			fec->ec = cpu_to_be32(wl_e->ec);
+
+			used_peb_count++;
+			fm_pos += sizeof(*fec);
+			ubi_assert(fm_pos <= ubi->fm_size);
+		}
+	}
 	fmh->used_peb_count = cpu_to_be32(used_peb_count);
 
 	for (node = rb_first(&ubi->scrub); node; node = rb_next(node)) {