UBI: recover_peb and power cut safety

[Richard Weinberger]

Forgot to CC Artem.

On Thu, Jun 16, 2016 at 11:46 AM, Richard Weinberger
<richard.weinberger at gmail.com> wrote:
> Jörg,
>
> On Thu, Jun 16, 2016 at 10:37 AM, Jörg Pfähler <pfaehler at isse.de> wrote:
>> Hi,
>>
>> I would greatly appreciate some clarification with respect to power cut safety
>> during writing of an erase block in UBI, specifically power cut safety of
>> recover_peb.
>>
>> During a normal write operation (ubi_eba_write_leb in mtd/ubi/eba.c) UBI tries
>> to move the contents of the block (and the new contents) to a new location via
>> recover_peb, if the write fails. However, recover_peb does not seem to use the
>> capability to exchange the (logical) block atomically (as
>> ubi_eba_atomic_leb_change in mtd/ubi/eba.c does). Specifically, it does not
>> seem to write the amount of data and its checksum to the VID header. Thus, if
>> the system crashes in the middle of recover_peb before the old/broken block
>> could be erased, we are left with a newer version of the block (the sequence
>> number in the header is increased by recover_peb), but without having moved
>> all the contents of the old block. This would obviously lead to data loss.
>> Thus, It seems to me that recover_peb (and therefore ubi_eba_write_leb) is not
>> power cut safe or is there some other mechanism distinct from the one used by
>> ubi_eba_atomic_leb_change to achieve this? If not I would suggest using
>> ubi_eba_atomic_leb_change in ubi_eba_write_leb instead of recover_peb.
>
> Hmm, you are right, if ubi_eba_write() is facing -EIO from the MTD driver we can
> lose the whole erase block upon power cut.
> So you found a bug. :-)
>
> Artem, can you tell more on this?
> I'd guess that recover_peb() is older than ubi_eba_atomic_leb_change() and
> therefore it was not used.
> And nobody noticed so far since the condition is hard to hit.
>
> That said, switching to ubi_eba_atomic_leb_change() seems like a good
> plan to me.
> Jörg, please send a patch and explain how you tested it.
>
> --
> Thanks,
> //richard



-- 
Thanks,
//richard