Ubi patch proposition for 3.10.y

Ubi patch proposition for 3.10.y

[jean-philippe francois]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

Hi,

I think commit 4b3e0a25... [1] (UBI: Call scan_all() with correct
offset in error case) should be added to 3.10.y stable branch.

It solves an ubifs corruption problem on my board. Edited bootlog for
each case are attached.

ubifs corruption can be easily reproduced on this board :
1) format nand partition and flash rootfs
2) record h264 stream on nand
3) scp a file from host pc to rootfs while 2 is ongoing
4) unplug power

So I am not an expert in UBI, but I guess from the commit description,
the first PEB are not only leaked, but necessary to build a valid UBI
map ?

Jean-Philippe François

[1] : http://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=4b3e0a25aa5518892ec8dbfafdd6a57789aa0c22

[-- Attachment #2: bootfail.txt --]
[-- Type: text/plain, Size: 8206 bytes --]

[    1.213104] enabling NAND BCH ecc with 8-bit correction
[    1.219024] ONFI param page 0 valid
[    1.222686] ONFI flash detected
[    1.225982] NAND device: Manufacturer ID: 0x2c, Chip ID: 0xac (Micron MT29F4G08ABBDAH4), 512MiB, page size: 2048, OOB size: 64
[    1.238006] 6 cmdlinepart partitions found on MTD device omap2-nand.0
[    1.244781] Creating 6 MTD partitions on "omap2-nand.0":
[    1.250396] 0x000000000000-0x000000040000 : "xl"
[    1.257324] 0x000000040000-0x000000100000 : "uboot"
[    1.264678] 0x000000100000-0x000000700000 : "boot1"
[    1.276458] 0x000000700000-0x000000d00000 : "boot2"
[    1.288085] 0x000000d00000-0x000001300000 : "boot3"
[    1.299743] 0x000001300000-0x000020000000 : "usr"

-- snip --

[    2.380676] UBI: default fastmap pool size: 195
[    2.385437] UBI: default fastmap WL pool size: 25
[    2.390472] UBI: attaching mtd5 to ubi0
[    2.680480] ------------[ cut here ]------------
[    2.685394] WARNING: at drivers/mtd/ubi/fastmap.c:827 ubi_attach_fastmap+0xae0/0xb50()
[    2.693725] Modules linked in:
[    2.696960] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.38-5 #2
[    2.703308] [<c00141c4>] (unwind_backtrace+0x0/0x128) from [<c00121a4>] (show_stack+0x20/0x24)
[    2.712371] [<c00121a4>] (show_stack+0x20/0x24) from [<c0415af0>] (dump_stack+0x20/0x28)
[    2.720916] [<c0415af0>] (dump_stack+0x20/0x28) from [<c0034fc0>] (warn_slowpath_common+0x5c/0x78)
[    2.730346] [<c0034fc0>] (warn_slowpath_common+0x5c/0x78) from [<c0035008>] (warn_slowpath_null+0x2c/0x34)
[    2.740509] [<c0035008>] (warn_slowpath_null+0x2c/0x34) from [<c02d0180>] (ubi_attach_fastmap+0xae0/0xb50)
[    2.750671] [<c02d0180>] (ubi_attach_fastmap+0xae0/0xb50) from [<c02d07bc>] (ubi_scan_fastmap+0x590/0x780)
[    2.760833] [<c02d07bc>] (ubi_scan_fastmap+0x590/0x780) from [<c02ccaa8>] (scan_fast+0x15c/0x1b8)
[    2.770172] [<c02ccaa8>] (scan_fast+0x15c/0x1b8) from [<c02cd0bc>] (ubi_attach+0x8c/0x234)
[    2.778869] [<c02cd0bc>] (ubi_attach+0x8c/0x234) from [<c02befac>] (ubi_attach_mtd_dev+0x2d4/0x5e0)
[    2.788391] [<c02befac>] (ubi_attach_mtd_dev+0x2d4/0x5e0) from [<c0636254>] (ubi_init+0x130/0x238)
[    2.797821] [<c0636254>] (ubi_init+0x130/0x238) from [<c00087c8>] (do_one_initcall+0x48/0xe8)
[    2.806823] [<c00087c8>] (do_one_initcall+0x48/0xe8) from [<c060cbe0>] (do_initcall_level+0x70/0xa0)
[    2.816436] [<c060cbe0>] (do_initcall_level+0x70/0xa0) from [<c060cc2c>] (do_initcalls+0x1c/0x28)
[    2.825744] [<c060cc2c>] (do_initcalls+0x1c/0x28) from [<c060cc60>] (do_basic_setup+0x28/0x30)
[    2.834808] [<c060cc60>] (do_basic_setup+0x28/0x30) from [<c060ccd0>] (kernel_init_freeable+0x54/0xe4)
[    2.844604] [<c060ccd0>] (kernel_init_freeable+0x54/0xe4) from [<c040a81c>] (kernel_init+0x18/0xf4)
[    2.854156] [<c040a81c>] (kernel_init+0x18/0xf4) from [<c000ed88>] (ret_from_fork+0x14/0x20)
[    2.863006] ---[ end trace 60c8c847091e2c43 ]---
[    2.867889] UBI error: ubi_scan_fastmap: Attach by fastmap failed, doing a full scan!
[    6.252258] UBI: scanning is finished
[    6.270050] UBI assert failed in ubi_wl_init at 1971 (pid 1)
[    6.276000] CPU: 0 PID: 1 Comm: swapper Tainted: G        W    3.10.38-5 #2
[    6.283386] [<c00141c4>] (unwind_backtrace+0x0/0x128) from [<c00121a4>] (show_stack+0x20/0x24)
[    6.292480] [<c00121a4>] (show_stack+0x20/0x24) from [<c0415af0>] (dump_stack+0x20/0x28)
[    6.301025] [<c0415af0>] (dump_stack+0x20/0x28) from [<c02ca53c>] (ubi_wl_init+0x498/0x5a8)
[    6.309814] [<c02ca53c>] (ubi_wl_init+0x498/0x5a8) from [<c02cd188>] (ubi_attach+0x158/0x234)
[    6.318786] [<c02cd188>] (ubi_attach+0x158/0x234) from [<c02befac>] (ubi_attach_mtd_dev+0x2d4/0x5e0)
[    6.328430] [<c02befac>] (ubi_attach_mtd_dev+0x2d4/0x5e0) from [<c0636254>] (ubi_init+0x130/0x238)
[    6.337860] [<c0636254>] (ubi_init+0x130/0x238) from [<c00087c8>] (do_one_initcall+0x48/0xe8)
[    6.346832] [<c00087c8>] (do_one_initcall+0x48/0xe8) from [<c060cbe0>] (do_initcall_level+0x70/0xa0)
[    6.356445] [<c060cbe0>] (do_initcall_level+0x70/0xa0) from [<c060cc2c>] (do_initcalls+0x1c/0x28)
[    6.365753] [<c060cc2c>] (do_initcalls+0x1c/0x28) from [<c060cc60>] (do_basic_setup+0x28/0x30)
[    6.374816] [<c060cc60>] (do_basic_setup+0x28/0x30) from [<c060ccd0>] (kernel_init_freeable+0x54/0xe4)
[    6.384643] [<c060ccd0>] (kernel_init_freeable+0x54/0xe4) from [<c040a81c>] (kernel_init+0x18/0xf4)
[    6.394165] [<c040a81c>] (kernel_init+0x18/0xf4) from [<c000ed88>] (ret_from_fork+0x14/0x20)
[    6.406036] UBI: attached mtd5 (name "usr", size 493 MiB) to ubi0
[    6.412567] UBI: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
[    6.419708] UBI: min./max. I/O unit sizes: 2048/2048, sub-page size 512
[    6.426666] UBI: VID header offset: 2048 (aligned 2048), data offset: 4096
[    6.433868] UBI: good PEBs: 3944, bad PEBs: 0, corrupted PEBs: 0
[    6.440185] UBI: user volume: 1, internal volumes: 1, max. volumes count: 128
[    6.447692] UBI: max/mean erase counter: 20/10, WL threshold: 4096, image sequence number: 16361325
[    6.457214] UBI: available PEBs: 0, total reserved PEBs: 3944, PEBs reserved for bad PEB handling: 80
[    6.466918] UBI: background thread "ubi_bgt0d" started, PID 561
[    6.474151] rtc-s35390a 2-0030: setting system clock to 2014-08-29 09:20:14 UTC (1409304014)
[    6.483459] Warning: unable to open an initial console.
[    6.492065] Freeing unused kernel memory: 2040K (c060c000 - c080a000)
[    6.566101] UBIFS: background thread "ubifs_bgt0_0" started, PID 573
[    6.609222] UBIFS: recovery needed
[    6.664886] UBI error: ubi_update_fastmap: could not find any anchor PEB
[    6.671966] UBI warning: ubi_update_fastmap: Unable to write new fastmap, err=-28
[    6.776702] UBIFS: recovery completed
[    6.780700] UBIFS: mounted UBI device 0, volume 0, name "user"
[    6.786834] UBIFS: LEB size: 126976 bytes (124 KiB), min./max. I/O unit sizes: 2048 bytes/2048 bytes
[    6.796447] UBIFS: FS size: 487841792 bytes (465 MiB, 3842 LEBs), journal size 24379392 bytes (23 MiB, 192 LEBs)
[    6.807159] UBIFS: reserved for root: 4952683 bytes (4836 KiB)
[    6.813262] UBIFS: media format: w4/r0 (latest is w4/r0), UUID 87231488-CF6D-4EEF-84A9-8A8DF44D98DB, small LPT model
[    6.825988] UBIFS error (pid 571): ubifs_read_node: bad node type (255 but expected 0)
[    6.834350] UBIFS error (pid 571): ubifs_read_node: bad node at LEB 739:118432, LEB mapping status 0
[    6.843933] Not a node, first 24 bytes:
[    6.847808] 00000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff                          ........................
[    6.861511] CPU: 0 PID: 571 Comm: mount Tainted: G        W    3.10.38-5 #2
[    6.868896] [<c00141c4>] (unwind_backtrace+0x0/0x128) from [<c00121a4>] (show_stack+0x20/0x24)
[    6.877960] [<c00121a4>] (show_stack+0x20/0x24) from [<c0415af0>] (dump_stack+0x20/0x28)
[    6.886474] [<c0415af0>] (dump_stack+0x20/0x28) from [<c01d0220>] (ubifs_read_node+0x2bc/0x2f8)
[    6.895629] [<c01d0220>] (ubifs_read_node+0x2bc/0x2f8) from [<c01f239c>] (ubifs_tnc_read_node+0x7c/0x208)
[    6.905700] [<c01f239c>] (ubifs_tnc_read_node+0x7c/0x208) from [<c01d4580>] (ubifs_tnc_locate+0xd8/0x1a4)
[    6.915771] [<c01d4580>] (ubifs_tnc_locate+0xd8/0x1a4) from [<c01cb5f8>] (ubifs_iget+0xcc/0x680)
[    6.925018] [<c01cb5f8>] (ubifs_iget+0xcc/0x680) from [<c01cbd1c>] (ubifs_fill_super.isra.7+0x170/0x1e4)
[    6.934997] [<c01cbd1c>] (ubifs_fill_super.isra.7+0x170/0x1e4) from [<c01cbf28>] (ubifs_mount+0x198/0x254)
[    6.945159] [<c01cbf28>] (ubifs_mount+0x198/0x254) from [<c00e0038>] (mount_fs+0x24/0xd4)
[    6.953796] [<c00e0038>] (mount_fs+0x24/0xd4) from [<c00f9e74>] (vfs_kern_mount+0x5c/0xd0)
[    6.962493] [<c00f9e74>] (vfs_kern_mount+0x5c/0xd0) from [<c00fb068>] (do_new_mount+0x9c/0x110)
[    6.971649] [<c00fb068>] (do_new_mount+0x9c/0x110) from [<c00fc0ac>] (do_mount+0x17c/0x1bc)
[    6.980438] [<c00fc0ac>] (do_mount+0x17c/0x1bc) from [<c00fc180>] (SyS_mount+0x94/0xc8)
[    6.988861] [<c00fc180>] (SyS_mount+0x94/0xc8) from [<c000ecc0>] (ret_fast_syscall+0x0/0x48)
[    6.997741] UBIFS error (pid 571): ubifs_iget: failed to read inode 1, error -22
[    7.005645] UBIFS: background thread "ubifs_bgt0_0" stops

[-- Attachment #3: bootok.txt --]
[-- Type: text/plain, Size: 4651 bytes --]

[    1.223297] NAND device: Manufacturer ID: 0x2c, Chip ID: 0xac (Micron MT29F4G08ABBDAH4), 512MiB, page size: 2048, OOB size: 64
[    1.235321] 6 cmdlinepart partitions found on MTD device omap2-nand.0
[    1.242095] Creating 6 MTD partitions on "omap2-nand.0":
[    1.247711] 0x000000000000-0x000000040000 : "xl"
[    1.254669] 0x000000040000-0x000000100000 : "uboot"
[    1.261993] 0x000000100000-0x000000700000 : "boot1"
[    1.273803] 0x000000700000-0x000000d00000 : "boot2"
[    1.285400] 0x000000d00000-0x000001300000 : "boot3"
[    1.297058] 0x000001300000-0x000020000000 : "usr"
--snip--
[    2.377929] UBI: default fastmap pool size: 195
[    2.382690] UBI: default fastmap WL pool size: 25
[    2.387725] UBI: attaching mtd5 to ubi0
[    2.678344] ------------[ cut here ]------------
[    2.683227] WARNING: at drivers/mtd/ubi/fastmap.c:827 ubi_attach_fastmap+0xae0/0xb50()
[    2.691558] Modules linked in:
[    2.694793] CPU: 0 PID: 1 Comm: swapper Not tainted 3.10.38-5 #5
[    2.701141] [<c00141c4>] (unwind_backtrace+0x0/0x128) from [<c00121a4>] (show_stack+0x20/0x24)
[    2.710235] [<c00121a4>] (show_stack+0x20/0x24) from [<c0415b00>] (dump_stack+0x20/0x28)
[    2.718749] [<c0415b00>] (dump_stack+0x20/0x28) from [<c0034fc0>] (warn_slowpath_common+0x5c/0x78)
[    2.728179] [<c0034fc0>] (warn_slowpath_common+0x5c/0x78) from [<c0035008>] (warn_slowpath_null+0x2c/0x34)
[    2.738342] [<c0035008>] (warn_slowpath_null+0x2c/0x34) from [<c02d0190>] (ubi_attach_fastmap+0xae0/0xb50)
[    2.748504] [<c02d0190>] (ubi_attach_fastmap+0xae0/0xb50) from [<c02d07cc>] (ubi_scan_fastmap+0x590/0x780)
[    2.758666] [<c02d07cc>] (ubi_scan_fastmap+0x590/0x780) from [<c02ccaa8>] (scan_fast+0x15c/0x1b8)
[    2.768005] [<c02ccaa8>] (scan_fast+0x15c/0x1b8) from [<c02cd0bc>] (ubi_attach+0x8c/0x244)
[    2.776702] [<c02cd0bc>] (ubi_attach+0x8c/0x244) from [<c02befac>] (ubi_attach_mtd_dev+0x2d4/0x5e0)
[    2.786254] [<c02befac>] (ubi_attach_mtd_dev+0x2d4/0x5e0) from [<c0636254>] (ubi_init+0x130/0x238)
[    2.795684] [<c0636254>] (ubi_init+0x130/0x238) from [<c00087c8>] (do_one_initcall+0x48/0xe8)
[    2.804656] [<c00087c8>] (do_one_initcall+0x48/0xe8) from [<c060cbe0>] (do_initcall_level+0x70/0xa0)
[    2.814300] [<c060cbe0>] (do_initcall_level+0x70/0xa0) from [<c060cc2c>] (do_initcalls+0x1c/0x28)
[    2.823638] [<c060cc2c>] (do_initcalls+0x1c/0x28) from [<c060cc60>] (do_basic_setup+0x28/0x30)
[    2.832672] [<c060cc60>] (do_basic_setup+0x28/0x30) from [<c060ccd0>] (kernel_init_freeable+0x54/0xe4)
[    2.842468] [<c060ccd0>] (kernel_init_freeable+0x54/0xe4) from [<c040a82c>] (kernel_init+0x18/0xf4)
[    2.852020] [<c040a82c>] (kernel_init+0x18/0xf4) from [<c000ed88>] (ret_from_fork+0x14/0x20)
[    2.860870] ---[ end trace e686b8b6b97a6313 ]---
[    2.865753] UBI error: ubi_scan_fastmap: Attach by fastmap failed, doing a full scan!
[    6.306152] UBI: scanning is finished
[    6.326934] UBI: attached mtd5 (name "usr", size 493 MiB) to ubi0
[    6.333343] UBI: PEB size: 131072 bytes (128 KiB), LEB size: 126976 bytes
[    6.340576] UBI: min./max. I/O unit sizes: 2048/2048, sub-page size 512
[    6.347564] UBI: VID header offset: 2048 (aligned 2048), data offset: 4096
[    6.354797] UBI: good PEBs: 3944, bad PEBs: 0, corrupted PEBs: 0
[    6.361083] UBI: user volume: 1, internal volumes: 1, max. volumes count: 128
[    6.368591] UBI: max/mean erase counter: 36/10, WL threshold: 4096, image sequence number: 16361325
[    6.378112] UBI: available PEBs: 0, total reserved PEBs: 3944, PEBs reserved for bad PEB handling: 80
[    6.387817] UBI: background thread "ubi_bgt0d" started, PID 561
[    6.394958] rtc-s35390a 2-0030: setting system clock to 2014-08-29 11:55:38 UTC (1409313338)
[    6.404815] Warning: unable to open an initial console.
[    6.413452] Freeing unused kernel memory: 2040K (c060c000 - c080a000)
[    6.483428] UBIFS: background thread "ubifs_bgt0_0" started, PID 573
[    6.530151] UBIFS: recovery needed
[    6.803771] UBIFS: recovery completed
[    6.807800] UBIFS: mounted UBI device 0, volume 0, name "user"
[    6.813934] UBIFS: LEB size: 126976 bytes (124 KiB), min./max. I/O unit sizes: 2048 bytes/2048 bytes
[    6.823547] UBIFS: FS size: 487841792 bytes (465 MiB, 3842 LEBs), journal size 24379392 bytes (23 MiB, 192 LEBs)
[    6.834228] UBIFS: reserved for root: 4952683 bytes (4836 KiB)
[    6.840362] UBIFS: media format: w4/r0 (latest is w4/r0), UUID 87231488-CF6D-4EEF-84A9-8A8DF44D98DB, small LPT model
[    9.225952] atsha204 2-0064: sha204p_send_command : xfer failed, err = -121
[    9.233551] entering sha204p_wakeup
[   13.158477] CMEMK module: built on Aug 28 2014 at 14:01:34

Re: Ubi patch proposition for 3.10.y

[Richard Weinberger]

Am 29.08.2014 14:26, schrieb jean-philippe francois:
> I think commit 4b3e0a25... [1] (UBI: Call scan_all() with correct
> offset in error case) should be added to 3.10.y stable branch.
> 
> It solves an ubifs corruption problem on my board. Edited bootlog for
> each case are attached.
> 
> ubifs corruption can be easily reproduced on this board :
> 1) format nand partition and flash rootfs
> 2) record h264 stream on nand
> 3) scp a file from host pc to rootfs while 2 is ongoing
> 4) unplug power

Hmm, I think this is a good point to bite the bullet and backport all
UBI fastmap related fixes to -stable.

Thanks,
//richard

Re: Ubi patch proposition for 3.10.y

[Greg KH]

On Mon, Sep 01, 2014 at 11:51:11PM +0200, Richard Weinberger wrote:
> Am 29.08.2014 14:26, schrieb jean-philippe francois:
> > I think commit 4b3e0a25... [1] (UBI: Call scan_all() with correct
> > offset in error case) should be added to 3.10.y stable branch.
> > 
> > It solves an ubifs corruption problem on my board. Edited bootlog for
> > each case are attached.
> > 
> > ubifs corruption can be easily reproduced on this board :
> > 1) format nand partition and flash rootfs
> > 2) record h264 stream on nand
> > 3) scp a file from host pc to rootfs while 2 is ongoing
> > 4) unplug power
> 
> Hmm, I think this is a good point to bite the bullet and backport all
> UBI fastmap related fixes to -stable.

And what exactly would those commits be?

thanks,

greg k-h

Re: Ubi patch proposition for 3.10.y

[Richard Weinberger]

Am 02.09.2014 00:02, schrieb Greg KH:
> On Mon, Sep 01, 2014 at 11:51:11PM +0200, Richard Weinberger wrote:
>> Am 29.08.2014 14:26, schrieb jean-philippe francois:
>>> I think commit 4b3e0a25... [1] (UBI: Call scan_all() with correct
>>> offset in error case) should be added to 3.10.y stable branch.
>>>
>>> It solves an ubifs corruption problem on my board. Edited bootlog for
>>> each case are attached.
>>>
>>> ubifs corruption can be easily reproduced on this board :
>>> 1) format nand partition and flash rootfs
>>> 2) record h264 stream on nand
>>> 3) scp a file from host pc to rootfs while 2 is ongoing
>>> 4) unplug power
>>
>> Hmm, I think this is a good point to bite the bullet and backport all
>> UBI fastmap related fixes to -stable.

Actually I meant, I need to sit down and go through the changelog and locate
them all. :)

> And what exactly would those commits be?

There you go:

e8c235b UBI: init_volumes: Ignore volumes with no LEBs
44305eb UBI: fastmap: do not miss bit-flips
604b592 UBI: fix rb_tree node comparison in add_map
5547fec UBI: fix some use after free bugs
fe24c6e UBI: Fix memory leak in ubi_attach_fastmap() error path
c22301a UBI: fastmap: fix backward compatibility with image_seq
4b3e0a2 UBI: Call scan_all() with correct offset in error case
f240dca UBI: Fix error path in scan_pool()
fb10e4d UBI: fix refill_wl_user_pool()
8930fa5 UBI: Fix invalidate_fastmap()

Thanks,
//richard

Re: Ubi patch proposition for 3.10.y

[Artem Bityutskiy]

On Thu, 2014-09-04 at 00:08 +0200, Richard Weinberger wrote:
> e8c235b UBI: init_volumes: Ignore volumes with no LEBs
> 44305eb UBI: fastmap: do not miss bit-flips
> 604b592 UBI: fix rb_tree node comparison in add_map
> 5547fec UBI: fix some use after free bugs
> fe24c6e UBI: Fix memory leak in ubi_attach_fastmap() error path
> c22301a UBI: fastmap: fix backward compatibility with image_seq
> 4b3e0a2 UBI: Call scan_all() with correct offset in error case
> f240dca UBI: Fix error path in scan_pool()
> fb10e4d UBI: fix refill_wl_user_pool()
> 8930fa5 UBI: Fix invalidate_fastmap()

The entire "fastmap" feature is classified as "experimental", why would
we need to be submit these to stable? I mean, we do not treat the
feature as bullet proof anyway yet.

-- 
Best Regards,
Artem Bityutskiy

Re: Ubi patch proposition for 3.10.y

[Richard Weinberger]

Artem,

Am 08.09.2014 11:47, schrieb Artem Bityutskiy:
> On Thu, 2014-09-04 at 00:08 +0200, Richard Weinberger wrote:
>> e8c235b UBI: init_volumes: Ignore volumes with no LEBs
>> 44305eb UBI: fastmap: do not miss bit-flips
>> 604b592 UBI: fix rb_tree node comparison in add_map
>> 5547fec UBI: fix some use after free bugs
>> fe24c6e UBI: Fix memory leak in ubi_attach_fastmap() error path
>> c22301a UBI: fastmap: fix backward compatibility with image_seq
>> 4b3e0a2 UBI: Call scan_all() with correct offset in error case
>> f240dca UBI: Fix error path in scan_pool()
>> fb10e4d UBI: fix refill_wl_user_pool()
>> 8930fa5 UBI: Fix invalidate_fastmap()
> 
> The entire "fastmap" feature is classified as "experimental", why would
> we need to be submit these to stable? I mean, we do not treat the
> feature as bullet proof anyway yet.

but people want to test it.
And we can get Fastmap only stable by having users.
As you know embedded folks tend to use rather old kernels.
Let's give them a chance to use the most current version of Fastmap on these kernels.

Thanks,
//richard

Re: Ubi patch proposition for 3.10.y

[Artem Bityutskiy]

On Fri, 2014-08-29 at 14:26 +0200, jean-philippe francois wrote:
> Hi,
> 
> I think commit 4b3e0a25... [1] (UBI: Call scan_all() with correct
> offset in error case) should be added to 3.10.y stable branch.

This is the "fastmap" fix, and fastmap is called experimental. We do not
have enough confidence it is production ready. Specifically, I'd like to
hear someone doing extensive power-cut testing with this feature. The
power-cut tolerance is one of the "selling features" of UBI/UBIFS, after
all.

Therefore I never added fastmap fixes to the stable queue.

But if someone is willing to put all the fastmap fixes together, add the
"stable tags" with the right kernel version "markings", and test for few
older kernels, then I will recommend them to be included to the stable.
Although I am not sure the stable maintainers would like accept them.

But I do not recommend adding this single patch to the stable queue.

But better, if people started paying more attention to "fastmap", we may
agree that from now on we are careful about the sending the fixes to the
stable queue.

Thanks!

-- 
Best Regards,
Artem Bityutskiy

Re: Ubi patch proposition for 3.10.y

[Willy Tarreau]

Hi Artem,

On Mon, Sep 08, 2014 at 12:58:38PM +0300, Artem Bityutskiy wrote:
> On Fri, 2014-08-29 at 14:26 +0200, jean-philippe francois wrote:
> > Hi,
> > 
> > I think commit 4b3e0a25... [1] (UBI: Call scan_all() with correct
> > offset in error case) should be added to 3.10.y stable branch.
> 
> This is the "fastmap" fix, and fastmap is called experimental. We do not
> have enough confidence it is production ready. Specifically, I'd like to
> hear someone doing extensive power-cut testing with this feature. The
> power-cut tolerance is one of the "selling features" of UBI/UBIFS, after
> all.
> 
> Therefore I never added fastmap fixes to the stable queue.
> 
> But if someone is willing to put all the fastmap fixes together, add the
> "stable tags" with the right kernel version "markings", and test for few
> older kernels, then I will recommend them to be included to the stable.
> Although I am not sure the stable maintainers would like accept them.
> 
> But I do not recommend adding this single patch to the stable queue.
> 
> But better, if people started paying more attention to "fastmap", we may
> agree that from now on we are careful about the sending the fixes to the
> stable queue.

Personally, I think that whatever feature provided in a kernel release is
subject to being used and deserves its fixes. There are some cases where
we *know* that some features are not used (eg: when they don't work without
a lot of patching or tweaking), but if they seem to work for end users, they
are likely to be used.

Just my two cents,
Willy

Re: Ubi patch proposition for 3.10.y

[Richard Genoud]

2014-09-08 12:04 GMT+02:00 Willy Tarreau <w@1wt.eu>:
> Hi Artem,
>
> On Mon, Sep 08, 2014 at 12:58:38PM +0300, Artem Bityutskiy wrote:
>> On Fri, 2014-08-29 at 14:26 +0200, jean-philippe francois wrote:
>> > Hi,
>> >
>> > I think commit 4b3e0a25... [1] (UBI: Call scan_all() with correct
>> > offset in error case) should be added to 3.10.y stable branch.
>>
>> This is the "fastmap" fix, and fastmap is called experimental. We do not
>> have enough confidence it is production ready. Specifically, I'd like to
>> hear someone doing extensive power-cut testing with this feature. The
>> power-cut tolerance is one of the "selling features" of UBI/UBIFS, after
>> all.
>>
>> Therefore I never added fastmap fixes to the stable queue.
>>
>> But if someone is willing to put all the fastmap fixes together, add the
>> "stable tags" with the right kernel version "markings", and test for few
>> older kernels, then I will recommend them to be included to the stable.
>> Although I am not sure the stable maintainers would like accept them.
>>
>> But I do not recommend adding this single patch to the stable queue.
>>
>> But better, if people started paying more attention to "fastmap", we may
>> agree that from now on we are careful about the sending the fixes to the
>> stable queue.
>
> Personally, I think that whatever feature provided in a kernel release is
> subject to being used and deserves its fixes. There are some cases where
> we *know* that some features are not used (eg: when they don't work without
> a lot of patching or tweaking), but if they seem to work for end users, they
> are likely to be used.
>
> Just my two cents,
> Willy

And fastmap is a really sexy feature...
I couldn't resist adding it to my project.
We are not in production yet, but we will be soon.
For now, I'm letting end users playing with it. That's a good test usually :).

And for the record, I'm stuck with 3.14.18 for now, so I'll welcome
fixes on fastmap.


Richard.

Re: Ubi patch proposition for 3.10.y

[Artem Bityutskiy]

On Mon, 2014-09-08 at 12:04 +0200, Willy Tarreau wrote:
> > But better, if people started paying more attention to "fastmap", we may
> > agree that from now on we are careful about the sending the fixes to the
> > stable queue.
> 
> Personally, I think that whatever feature provided in a kernel release is
> subject to being used and deserves its fixes. There are some cases where
> we *know* that some features are not used (eg: when they don't work without
> a lot of patching or tweaking), but if they seem to work for end users, they
> are likely to be used.

Well, adding stable tags is does not come for free. Whey you add a
stable tag, you need to make sure it applies to the older kernel, and
you need to also pick the possible dependencies. You need test it with
older kernel.

While in general what you say makes sense, for this specific "fastmap"
case the maintainers (me and Richard) did not think the overhead is
worth the benefits.

If now the situation has changed, we may start being more careful.

-- 
Best Regards,
Artem Bityutskiy

Re: Ubi patch proposition for 3.10.y

[Richard Weinberger]

Am 08.09.2014 11:58, schrieb Artem Bityutskiy:
> On Fri, 2014-08-29 at 14:26 +0200, jean-philippe francois wrote:
>> Hi,
>>
>> I think commit 4b3e0a25... [1] (UBI: Call scan_all() with correct
>> offset in error case) should be added to 3.10.y stable branch.
> 
> This is the "fastmap" fix, and fastmap is called experimental. We do not
> have enough confidence it is production ready. Specifically, I'd like to
> hear someone doing extensive power-cut testing with this feature. The
> power-cut tolerance is one of the "selling features" of UBI/UBIFS, after
> all.
> 
> Therefore I never added fastmap fixes to the stable queue.
> 
> But if someone is willing to put all the fastmap fixes together, add the
> "stable tags" with the right kernel version "markings", and test for few
> older kernels, then I will recommend them to be included to the stable.
> Although I am not sure the stable maintainers would like accept them.

I'll take care of that from now on.

Thanks,
//richard

Re: Ubi patch proposition for 3.10.y

[Willy Tarreau]

On Mon, Sep 08, 2014 at 01:13:41PM +0300, Artem Bityutskiy wrote:
> Well, adding stable tags is does not come for free. Whey you add a
> stable tag, you need to make sure it applies to the older kernel, and
> you need to also pick the possible dependencies. You need test it with
> older kernel.
> 
> While in general what you say makes sense, for this specific "fastmap"
> case the maintainers (me and Richard) did not think the overhead is
> worth the benefits.

I understand and am not pressuring anyone to do the backport. Sometimes if
something appears to be clearly broken, for example to the point where data
corruption may result from the use of any feature, a patch to disable the
feature or make it depend on CONFIG_BROKEN might be good as well to warn
users they must not use it because it's too broken.

Regards,
Willy

Re: Ubi patch proposition for 3.10.y

[jean-philippe francois]

2014-09-08 12:13 GMT+02:00 Artem Bityutskiy <dedekind1@gmail.com>:
> On Mon, 2014-09-08 at 12:04 +0200, Willy Tarreau wrote:
>> > But better, if people started paying more attention to "fastmap", we may
>> > agree that from now on we are careful about the sending the fixes to the
>> > stable queue.
>>
>> Personally, I think that whatever feature provided in a kernel release is
>> subject to being used and deserves its fixes. There are some cases where
>> we *know* that some features are not used (eg: when they don't work without
>> a lot of patching or tweaking), but if they seem to work for end users, they
>> are likely to be used.
>
> Well, adding stable tags is does not come for free. Whey you add a
> stable tag, you need to make sure it applies to the older kernel, and
> you need to also pick the possible dependencies. You need test it with
> older kernel.
>
> While in general what you say makes sense, for this specific "fastmap"
> case the maintainers (me and Richard) did not think the overhead is
> worth the benefits.
>
> If now the situation has changed, we may start being more careful.
>
Well for the original commit case :
- it applies
- it has been tested (by me at least)
- it solves a power cut fs corruption issue.

You ask for power cut testing, but when presented with the results,
patch is dismissed because I am foolish enough to use a long term
stable kernel, and not jump version each time a new kernel comes up.

Hopefully, dts will greatly help with migrating kernel, but
unoftunately, our camera was developped at the wrong moment (no more
board file accepted, yet no dts support for vital peripheral like
ISP).

It started with one patch, and now tens of them are mentionned, so I
understand your concern.

Regards,
Jean-Philippe François

> Best Regards,
> Artem Bityutskiy
>