Patchwork [v3,0/6] UBI: add max_beb_per1024 parameter / ioctl

login
register
mail settings
Submitter Richard Genoud
Date Aug. 31, 2012, 2:46 p.m.
Message ID <CACQ1gAiFQfAbKVRfhjy-K31MJP9YchzxmmCo4FJECoKuudeBtw@mail.gmail.com>
Download mbox | patch
Permalink /patch/180985/
State New
Headers show

Comments

Richard Genoud - Aug. 31, 2012, 2:46 p.m.
2012/8/23 Richard Genoud <richard.genoud@gmail.com>:
> I could do some more tests with the final version.
I did some test:
- new ubiattacht/detach with old kernel
- old ubiattacht/detach/mkvol/format/ ubifs mount with new kernel
- new ubi* with new kernel.
- command line ubi.mtd=...
I ran into the case where I asked for 20 PEB reserved for bad PEB handling:
UBI warning: print_rsvd_warning: cannot reserve enough PEBs for bad
PEB handling, reserved 10, need 20
and I did a ubirmvol on a small volume:
# ubirmvol /dev/ubi0 -N toto2
UBI: reserved more 10 PEBs for bad PEB handling
=> that's great, no need to detach and then re-attach

I tested also some values at limits, it seems all right.

BUT, I ran into a bug, I don't know if it's my kernel (as I've got
quite a lot of patches ahead of 3.6 to get my board run), but it's
kind of nasty.
It's not related to this patch serie, I could trigger it with a
3.6-rc1 kernel (and my board-patches).
I get some memory crashing when I do :
flash_erase /dev/mtd2 0 160 (it's a whole mtd partition)
ubiattach -m 2
After that, I can have a OOPS right away, but sometimes not.
but if I run someting (top for exemple) I get an oops then.
So, the memory is beeing corrupted somewhere, but I couldn't find out
exactly where.
But I found that if I comment out the lines :

As I can't test with older kernel, I reverted some commits that
touched drivers/mtd/ubi
I reverted all commits from 3.6-rc1 to
4415626732defb5a4567a0a757c7c5baae7ca846 (UBI: amend commentaries WRT
dtype)
and I still can trigger the oops.

Does someone can reproduce that ?

Best regards,
Richard
Artem Bityutskiy - Sept. 3, 2012, 8 a.m.
On Fri, 2012-08-31 at 16:46 +0200, Richard Genoud wrote:
> BUT, I ran into a bug, I don't know if it's my kernel (as I've got
> quite a lot of patches ahead of 3.6 to get my board run), but it's
> kind of nasty.

What is your kernel? Why don't you just pull the corresponding ubifs
backport tree?

> It's not related to this patch serie, I could trigger it with a
> 3.6-rc1 kernel (and my board-patches).
> I get some memory crashing when I do :
> flash_erase /dev/mtd2 0 160 (it's a whole mtd partition)
> ubiattach -m 2

Would you please be able to reproduce this with linux-ubi tree and also
send be the full oops?

> After that, I can have a OOPS right away, but sometimes not.
> but if I run someting (top for exemple) I get an oops then.
> So, the memory is beeing corrupted somewhere, but I couldn't find out
> exactly where.

If you could somehow reproduce this with nandsim, it would make it easy
for me to find the bug.

> But I found that if I comment out the lines :
> diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
> --- a/drivers/mtd/ubi/vtbl.c
> +++ b/drivers/mtd/ubi/vtbl.c
> @@ -346,8 +346,8 @@ retry:
>          */
>         err = ubi_scan_add_used(ubi, si, new_seb->pnum, new_seb->ec,
>                                 vid_hdr, 0);

We renamed all scan functions recently, so AFAICS you are not using the
latest UBI code-base. How about picking the latest back-port tree?

> +//     kfree(new_seb);
> +//     ubi_free_vid_hdr(ubi, vid_hdr);

This is strange, I think these 2 frees are all-right, I think the
root-cause is somewhere else.


> Does someone can reproduce that ?

It is difficult to say because you did not send the full oops.
Artem Bityutskiy - Sept. 3, 2012, 2:32 p.m.
On Fri, 2012-08-31 at 16:46 +0200, Richard Genoud wrote:
> diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
> --- a/drivers/mtd/ubi/vtbl.c
> +++ b/drivers/mtd/ubi/vtbl.c
> @@ -346,8 +346,8 @@ retry:
>          */
>         err = ubi_scan_add_used(ubi, si, new_seb->pnum, new_seb->ec,
>                                 vid_hdr, 0);
> +//     kfree(new_seb);
> +//     ubi_free_vid_hdr(ubi, vid_hdr);
>         return err;
> 
Richard, I've sent you the fix. I've tested it and it fixes the issue.
Switching to SLUB must be hiding it somehow. Thanks for reporting!
>
Artem Bityutskiy - Sept. 3, 2012, 2:33 p.m.
On Fri, 2012-08-31 at 16:46 +0200, Richard Genoud wrote:
> I tested also some values at limits, it seems all right.

Thanks Richard for testing. Would you be also energetic enough to do the
last step - some documentation for mtd-www?
Richard Genoud - Sept. 3, 2012, 2:45 p.m.
2012/9/3 Artem Bityutskiy <dedekind1@gmail.com>:
> On Fri, 2012-08-31 at 16:46 +0200, Richard Genoud wrote:
>> I tested also some values at limits, it seems all right.
>
> Thanks Richard for testing. Would you be also energetic enough to do the
> last step - some documentation for mtd-www?

yep, that's next in line !

Patch

diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
--- a/drivers/mtd/ubi/vtbl.c
+++ b/drivers/mtd/ubi/vtbl.c
@@ -346,8 +346,8 @@  retry:
         */
        err = ubi_scan_add_used(ubi, si, new_seb->pnum, new_seb->ec,
                                vid_hdr, 0);
+//     kfree(new_seb);
+//     ubi_free_vid_hdr(ubi, vid_hdr);
        return err;

 write_error:

I can't trigger an oops any more.