Re: JFFS2 locking issue on 3.14 + patches

[Michal Simek <monstr@monstr.eu>]

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

On 05/16/2014 08:10 AM, Brian Norris wrote:
> On Wed, Apr 30, 2014 at 07:43:08AM +0200, Michal Simek wrote:
>> On 04/29/2014 07:05 PM, Brian Norris wrote:
>>> On Tue, Apr 29, 2014 at 02:02:07PM +0200, Michal Simek wrote:
>>>> commit a445f784ae5558a3da680aa6b39ed53c95a551c1
>>>> Author: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>>>> Date:   Thu Aug 23 10:10:07 2012 +0300
>>>>
>>>>     JFFS2: fix unmount regression
>>>>
>>>>     This patch fixes regression introduced by
>>>>     "8bdc81c jffs2: get rid of jffs2_sync_super". We submit a delayed work in order
>>>>     to make sure the write-buffer is synchronized at some point. But we do not
>>>>     flush it when we unmount, which causes an oops when we unmount the file-system
>>>>     and then the delayed work is executed.
>>>>
>>>>     This patch fixes the issue by adding a "cancel_delayed_work_sync()" infocation
>>>>     in the '->sync_fs()' handler. This will make sure the delayed work is canceled
>>>>     on sync, unmount and re-mount. And because VFS always callse 'sync_fs()' before
>>>>     unmounting or remounting, this fixes the issue.
>>>>
>>>>     Reported-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>>>>     Cc: stable@vger.kernel.org [3.5+]
>>>>     Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>>>>     Tested-by: Ludovic Desroches <ludovic.desroches@atmel.com>
>>>>     Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
>>>
>>> Bisection turned you up here?
>>
>> I have looked at it more and JFFS2_FS_WRITEBUFFER is broken from this commit.
> 
> Write buffer support may or may not be necessary. What type of flash are
> you using?
> 
>> When you disable it is working till this commit on Microblaze.
>>
>> commit ac093f8d5e76be1f2654acfd7a59d339ba037654
>> Author:     Michael S. Tsirkin <mst@redhat.com>
>>
>>     microblaze: uaccess s/might_sleep/might_fault/
> 
> Hmm, that's only a debugging change. It only has an effect if you have
> CONFIG_PROVE_LOCKING or CONFIG_DEBUG_ATOMIC_SLEEP enabled. So that's not
> the issue.

Yes I know.

> 
>> And surprisingly the same change is causing problem on ARM.
>>
>> commit ad72907acd2943304c292ae36960bb66e6dc23c9
>> Author:     Will Deacon <will.deacon@arm.com>
>>
>>     ARM: 7528/1: uaccess: annotate [__]{get,put}_user functions with might_fault()
> 
> *Not* surprisingly; it's the same thing, where we're enabling a
> debugging feature. Assuming the might_fault() code is accurate, it
> simply means we can now discover a bug that already exists, right?

yes.

What is more problematic is that the issue have been reported
in past but probably never fixed.
http://lists.infradead.org/pipermail/linux-mtd/2013-June/047312.html
http://lists.infradead.org/pipermail/linux-mtd/2013-June/047316.html
http://lists.infradead.org/pipermail/linux-mtd/2013-February/045888.html
http://www.spinics.net/lists/linux-rt-users/msg07617.html

On v3.6 we didn't observe any problem but it can be just because of
not sufficient testing.

>> I have revert these 2 patches and disable JFFS2_FS_WRITEBUFFER
>> on the top of that 5 patches and everything seems to work.
>>
>> Then I have tried to disable debug locking features which I have ON
>> and I have ended up that disabling CONFIG_PROVE_LOCKING
>> and CRC problem is back again.
> 
> OK, so I don't think you've bisected properly. The might_fault() patches
> seem unlikely to be a real source of the problem. If anything, they may
> be helping to uncover it. But I do not know. I don't have very good
> information to go off of here.

I wouldn't say that I didn't bisected this properly. When commits
above are the first broken one that bisect was fine.
What wasn't sufficient was my tests because I wasn't able to reach
any problem before these commits.
Do you have written somewhere what tests should be run to do proper
FS testing? It can be the part of any other testing system like LTP.

Are there any standard tests which you are running on others FS?

>>> Honestly, I'm not too familiar with JFFS2. Maybe Artem can comment,
>>> since he authored this commit.
>>
>> Is someone doing any testing on JFFS2 at all?
>> From this debugging it looks like that JFFS2 have some serious issues
>> for quite a long time.
> 
> I do not know who is doing JFFS2 testing these days. Most people have
> moved to UBIFS, which gets better support. JFFS2 is mostly in
> maintenance mode, with little active work.

I am not surprised.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]