ubi: reset relevant globals in ubi_exit()
Subsystems (2)
PATCH 1/1 ubi: reset relevant globals in ubi_exit()
Before calling ubi_init() the U-Boot wrapper calls ubi_mtd_param_parse() to have the UBI driver add it to its mtd_dev_param[] array and increment mtd_devs. The first time ubi_init() is called, mtd_devs would be 1.
Before ubi_init() is called again (another partition is attached), ubi_mtd_param_parse() is called again, incrementing mtd_devs again (now 2). This results in ubi_init() now trying to attach the first partition and the second partition.
Fix this by adding a section at the end of ubi_exit() where we reset any globals that would need to be reset (in this case, just mtd_devs).
Test case:
$ ubi part <mtdname> ; ubi part <mtdname>
Before patch:
$ ubi part data UBI: attaching mtd1 to ubi0 UBI: scanning is finished UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0 [...] UBI: available PEBs: 0, total reserved PEBs: 32, [...] $ ubi part data UBI: detaching mtd1 from ubi0 UBI: mtd1 is detached from ubi0 UBI: attaching mtd1 to ubi0 [...] UBI: available PEBs: 0, total reserved PEBs: 32, [...] ** Note that this is where it tries to attach mtd1 again, fails, and ** then detaches everything as it errors out of ubi_init() UBI: detaching mtd1 from ubi0 UBI: mtd1 is detached from ubi0 UBI init error 17 $
After patch:
$ ubi part data UBI: attaching mtd1 to ubi0 UBI: scanning is finished UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0 [...] UBI: available PEBs: 0, total reserved PEBs: 32, [...] $ ubi part data UBI: detaching mtd1 from ubi0 UBI: mtd1 is detached from ubi0 UBI: attaching mtd1 to ubi0 UBI: scanning is finished UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0 [...] UBI: available PEBs: 0, total reserved PEBs: 32, [...] $
--- drivers/mtd/ubi/build.c | 4 ++++ 1 file changed, 4 insertions(+)
Not sure this is the best place to make the change, but it is one of the least obtrusive, IMO. Please Cc: me on any responses as it will go directly to my inbox!
drivers/mtd/ubi/build.c +4 -0
@@ -1384,6 +1384,10 @@ void ubi_exit(void)misc_deregister (&ubi_ctrl_cdev);class_remove_file (ubi_class, &ubi_version);class_destroy (ubi_class); +#ifdef __UBOOT__ +/* Reset any globals that the driver depends on being zeroed */ + mtd_devs =0 ; +#endif }module_exit (ubi_exit);
Signature · 1 line
2.1.1
PATCH 1/1 ubi: reset relevant globals in ubi_exit()
Hello Andrew,
Am 13.11.2014 18:05, schrieb Andrew Ruder:
Quoted 69 lines
Before calling
ubi_init()the U-Boot wrapper callsubi_mtd_param_parse()to have the UBI driver add it to its mtd_dev_param[] array and incrementmtd_devs. The first timeubi_init()is called,mtd_devswould be 1.Before
ubi_init()is called again (another partition is attached),ubi_mtd_param_parse()is called again, incrementingmtd_devsagain (now 2). This results inubi_init()now trying to attach the first partition and the second partition.Fix this by adding a section at the end of
ubi_exit()where we reset any globals that would need to be reset (in this case, justmtd_devs).Test case:
$ ubi part <mtdname> ; ubi part <mtdname>Before patch:
$ ubi part data UBI: attaching mtd1 to ubi0 UBI: scanning is finished UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0 [...] UBI: available PEBs: 0, total reserved PEBs: 32, [...] $ ubi part data UBI: detaching mtd1 from ubi0 UBI: mtd1 is detached from ubi0 UBI: attaching mtd1 to ubi0 [...] UBI: available PEBs: 0, total reserved PEBs: 32, [...] ** Note that this is where it tries to attach mtd1 again, fails, and ** then detaches everything as it errors out of ubi_init() UBI: detaching mtd1 from ubi0 UBI: mtd1 is detached from ubi0 UBI init error 17 $After patch:
$ ubi part data UBI: attaching mtd1 to ubi0 UBI: scanning is finished UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0 [...] UBI: available PEBs: 0, total reserved PEBs: 32, [...] $ ubi part data UBI: detaching mtd1 from ubi0 UBI: mtd1 is detached from ubi0 UBI: attaching mtd1 to ubi0 UBI: scanning is finished UBI: attached mtd1 (name "mtd=4", size 4 MiB) to ubi0 [...] UBI: available PEBs: 0, total reserved PEBs: 32, [...] $Signed-off-by Andrew Ruder <[email protected]>Cc Heiko Schocher <[email protected]>Cc Kyungmin Park <[email protected]>--- drivers/mtd/ubi/build.c | 4 ++++ 1 file changed, 4 insertions(+)Not sure this is the best place to make the change, but it is one of the least obtrusive, IMO. Please Cc: me on any responses as it will go directly to my inbox!
drivers/mtd/ubi/build.c +0
-0
drivers/mtd/ubi/build.c void ubi_exit(void) · line 1392
Quoted 9 lines
drivers/mtd/ubi/build.c +4
-0@@ -1384,6 +1384,10 @@ void ubi_exit(void)misc_deregister (&ubi_ctrl_cdev);class_remove_file (ubi_class, &ubi_version);class_destroy (ubi_class); +#ifdef __UBOOT__ +/* Reset any globals that the driver depends on being zeroed */ + mtd_devs =0 ; +#endif }module_exit (ubi_exit);
Good catch, but wondering, why this not poped up in my tests, as I did such a test ... Hmm... trying currently on the tqm5200s board on a cfi nor flash [1] shows no error for me. But I just activated ubi errormessages and broke the board ... so I must wait for a debugger :-(
Beside of this, your patch seems feasible, I test it soon.
bye,
Heiko
[1] log on the tqm5200s board:
=> mtdparts
device nor0 <fc000000.flash>, # parts = 7
#: name size offset mask_flags 0: firmware 0x00100000 0x00000000 0 1: dtb 0x00040000 0x00100000 0 2: kernel 0x00240000 0x00140000 0 3: small-fs 0x00280000 0x00380000 0 4: initrd 0x00200000 0x00600000 0 5: misc 0x00800000 0x00800000 0 6: big-fs 0x01000000 0x01000000 0
active partition: nor0,0 - (firmware) 0x00100000 @ 0x00000000
defaults:
mtdids : nor0=fc000000.flash
mtdparts: mtdparts=fc000000.flash:1m(firmware),256k(dtb),2304k(kernel),2560k(small-fs),2m(initrd),8m(misc),16m(big-fs) => ubi part misc
UBI: default fastmap pool size: 8
UBI: default fastmap WL pool size: 25
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: empty MTD device detected
UBI: attached mtd1 (name "mtd=5", size 8 MiB) to ubi0
UBI: PEB size: 262144 bytes (256 KiB), LEB size: 262016 bytes
UBI: min./max. I/O unit sizes: 1/1, sub-page size 1
UBI: VID header offset: 64 (aligned 64), data offset: 128
UBI: good PEBs: 32, bad PEBs: 0, corrupted PEBs: 0
UBI: user volume: 0, internal volumes: 1, max. volumes count: 128
UBI: max/mean erase counter: 1/0, WL threshold: 4096, image sequence number: 0
UBI: available PEBs: 26, total reserved PEBs: 6, PEBs reserved for bad PEB handling: 0
=> ubi info l
Volume information dump:
vol_id 2147479551
reserved_pebs 2
alignment 1
data_pad 0
vol_type 3
name_len 13
usable_leb_size 262016
used_ebs 2
used_bytes 524032
last_eb_bytes 2
corrupted 0
upd_marker 0
name layout volume=> ubi info
UBI: MTD device name: "mtd=5"
UBI: MTD device size: 8 MiB
UBI: physical eraseblock size: 262144 bytes (256 KiB)
UBI: logical eraseblock size: 262016 bytes
UBI: number of good PEBs: 32
UBI: number of bad PEBs: 0
UBI: smallest flash I/O unit: 1
UBI: VID header offset: 64 (aligned 64)
UBI: data offset: 128
UBI: max. allowed volumes: 128
UBI: wear-leveling threshold: 4096
UBI: number of internal volumes: 1
UBI: number of user volumes: 0
UBI: available PEBs: 26
UBI: total number of reserved PEBs: 6
UBI: number of PEBs reserved for bad PEB handling: 0
UBI: max/mean erase counter: 1/0
=> ubi create test 400000
Creating dynamic volume test of size 4194304 => ubi info l
Volume information dump:
vol_id 0
reserved_pebs 17
alignment 1
data_pad 0
vol_type 3
name_len 4
usable_leb_size 262016
used_ebs 17
used_bytes 4454272
last_eb_bytes 262016
corrupted 0
upd_marker 0
name test
Volume information dump:
vol_id 2147479551
reserved_pebs 2
alignment 1
data_pad 0
vol_type 3
name_len 13
usable_leb_size 262016
used_ebs 2
used_bytes 524032
last_eb_bytes 2
corrupted 0
upd_marker 0
name layout volume=> ubi part misc
UBI: detaching mtd1 from ubi0
UBI: mtd1 is detached from ubi0
UBI: default fastmap pool size: 8
UBI: default fastmap WL pool size: 25
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: attached mtd1 (name "mtd=5", size 8 MiB) to ubi0
UBI: PEB size: 262144 bytes (256 KiB), LEB size: 262016 bytes
UBI: min./max. I/O unit sizes: 1/1, sub-page size 1
UBI: VID header offset: 64 (aligned 64), data offset: 128
UBI: good PEBs: 32, bad PEBs: 0, corrupted PEBs: 0
UBI: user volume: 1, internal volumes: 1, max. volumes count: 128
UBI: max/mean erase counter: 2/1, WL threshold: 4096, image sequence number: 0
UBI: available PEBs: 9, total reserved PEBs: 23, PEBs reserved for bad PEB handling: 0 => ubi part misc
UBI: detaching mtd1 from ubi0
UBI: mtd1 is detached from ubi0
UBI: default fastmap pool size: 8
UBI: default fastmap WL pool size: 25
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: attached mtd1 (name "mtd=5", size 8 MiB) to ubi0
UBI: PEB size: 262144 bytes (256 KiB), LEB size: 262016 bytes
UBI: min./max. I/O unit sizes: 1/1, sub-page size 1
UBI: VID header offset: 64 (aligned 64), data offset: 128
UBI: good PEBs: 32, bad PEBs: 0, corrupted PEBs: 0
UBI: user volume: 1, internal volumes: 1, max. volumes count: 128
UBI: max/mean erase counter: 2/1, WL threshold: 4096, image sequence number: 0
UBI: available PEBs: 9, total reserved PEBs: 23, PEBs reserved for bad PEB handling: 0 => ubi part misc
UBI: detaching mtd1 from ubi0
UBI: mtd1 is detached from ubi0
UBI: default fastmap pool size: 8
UBI: default fastmap WL pool size: 25
UBI: attaching mtd1 to ubi0
UBI: scanning is finished
UBI: attached mtd1 (name "mtd=5", size 8 MiB) to ubi0
UBI: PEB size: 262144 bytes (256 KiB), LEB size: 262016 bytes
UBI: min./max. I/O unit sizes: 1/1, sub-page size 1
UBI: VID header offset: 64 (aligned 64), data offset: 128
UBI: good PEBs: 32, bad PEBs: 0, corrupted PEBs: 0
UBI: user volume: 1, internal volumes: 1, max. volumes count: 128
UBI: max/mean erase counter: 2/1, WL threshold: 4096, image sequence number: 0
UBI: available PEBs: 9, total reserved PEBs: 23, PEBs reserved for bad PEB handling: 0 =>
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 ubi: reset relevant globals in ubi_exit()
Quoted 2 lines · Heiko Schocher
Good catch, but wondering, why this not poped up in my tests, as I did such a test ...
Are you on 2014.10? I don't think this issue existed on the 2014.07-rc3 I was using earlier.
- Andy
PATCH 1/1 ubi: reset relevant globals in ubi_exit()
Hello Andrew,
Am 14.11.2014 14:31, schrieb Andrew Ruder:
Quoted 6 lines · Heiko Schocher
Quoted 2 lines
Good catch, but wondering, why this not poped up in my tests, as I did such a test ...
Are you on 2014.10? I don't think this issue existed on the 2014.07-rc3 I was using earlier.
Yes, I tested currently with current mainline ...
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
PATCH 1/1 ubi: reset relevant globals in ubi_exit()
Hello Andrew,
Am 17.11.2014 07:21, schrieb Heiko Schocher:
Quoted 11 lines
Hello Andrew,
Am 14.11.2014 14:31, schrieb Andrew Ruder:
Quoted 6 lines · Heiko Schocher
Quoted 2 lines
Good catch, but wondering, why this not poped up in my tests, as I did such a test ...
Are you on 2014.10? I don't think this issue existed on the 2014.07-rc3 I was using earlier.
Yes, I tested currently with current mainline ...
Could you please test on your hw with current ML? I could not see this problem, but maybe I have another setup ... thanks!
bye,
Heiko
Signature · 2 lines
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany