Re: jffs2: Excess summary entries

[Wei Fang <fangwei1@huawei.com>]

Hi Thomas,

On 2015/11/9 15:19, Wei Fang wrote:
> Hi Thomas,
> 
> On 2015/11/6 2:36, Thomas.Betker@rohde-schwarz.com wrote:
>> We ran into a problem with jffs2 where the filesystem became unusable 
>> after some specific MTD failures; summary was enabled, write buffering was 
>> disabled:
> 
> [snip...]
> 
>> Here are the solutions I have considered so far:
>>
>> The minimal solution would be to check *retlen == 0 in 
>> jffs2_flash_direct_writev() and jffs2_flash_direct_write() before running 
>> jffs2_sum_add_kvec().
>>
>>         if (jffs2_sum_active() && *retlen) {
>>                 ...
>>                 res = jffs2_sum_add_kvec(...)
>>                 ...
>>         }
>>
>> The general failure case, though, is (ret != 0 || *retlen != len), where 
>> 'ret' is the return code of mtd_writev(), and 'len' is the data size to be 
>> written. When write buffering is enabled, jffs2_flash_writev() in wbuf.c 
>> skips the summary entry in this case; perhaps we should do this in 
>> writev.c as well?
>>
>>         if (jffs2_sum_active() && !ret && *retlen == len) {
>>                 ...
>>                 res = jffs2_sum_add_kvec(...)
>>                 ...
>>         }
> 
> I prefer:
> 	if (jffs2_sum_active() && *retlen == len) {
> 		...
> 		res = jffs2_sum_add_kvec(...)
> 		...
> 	}
> 
> In the case that part of node has been written to flash, this whole node
> will be marked as dirty node, only in memory, not marked without
> JFFS2_NODE_ACCURATE on flash.
> 
> If the summary is stored when *retlen != len, there are two cases:
> 
> * In most case, another write with the same node info performs
>   successfully later, the node written partially before will be marked
>   as obsolete node when scan, and we won't read from it
> * This node is the newest node about this region, it will be treated
>   as normal node when scan, and we may read the data already corrupted.
>   Yet it won't break any rules of JFFS2 and lead to a muddle

I forgot there's crc check about the data, so in this case, this node
couldn't pass the crc check and will be marked as obsolete too :)

Thanks,
Wei

> The node written partially will be treated as normal node in a full scan
> routine too, so I think we should mark this node as dirty on flash in
> the case that *retlen != len.
> 
> Thanks,
> Wei
> 
>> I ran some quick tests, simulating write failures, and it seems that 
>> adding the summary entry doesn't harm when *retlen != 0 [so the minimal 
>> solution would suffice]. This is because the calling function will reserve 
>> the node space, marking it as dirty, and there is no confusion about 
>> unchecked space.
>>
>> On the other hand, running the same quick tests _without_ adding the 
>> summary entry didn't seem to harm either [so the general solution would 
>> work as well]. It is entirely possible that I have overlooked something, 
>> though.
>>
>> Any opinions on that? When in doubt, I would provide a patch for the 
>> minimal solution, changing as little as possible. However, it may make 
>> sense to go for the general solution to be consistent with write 
>> buffering.
>>
>> Best regards,
>> Thomas Betker
>>
>> ______________________________________________________
>> Linux MTD discussion mailing list
>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>>
>>