[JFFS2]The patch "jffs2: Fix lock acquisition order bug in jffs2_write_begin" introduces another dead lock bug.

[Thomas.Betker at rohde-schwarz.com]

Hello Chao:

> The full header of the patch is below:
> author Thomas Betker <thomas.betker at freenet.de> 2012-10-17 20:59:30 
(GMT)
> committer Artem Bityutskiy <artem.bityutskiy at linux.intel.com> 
> 2012-11-09 15:02:50 (GMT)
> commit 5ffd3412ae5536a4c57469cb8ea31887121dcb2e?(patch)
> tree 33c9d89eabf70c49e683a36be800be3b4582a358?/fs/jffs2
> parent 0131950ebd146b5e31508233352d6f4625af25b1?(diff)
> 
> jffs2: Fix lock acquisition order bug in jffs2_write_begin.
> 
> jffs2_write_begin first acquires the c->alloc_sem, then the page 
> lock. This causes an ABBA dead lock with jffs2_write_end, which 
> already have hold the page lock, then the c->alloc_sem:
> THREAD ONE:
> jffs2_write_begin
>  jffs2_reserve_space
>   mutex_lock(&c->alloc_sem) A
>  grab_cache_page_write_begin 
>   find_lock_page
>    lock_page(page) B
> THERAD TWO:
> jffs2_write_end  B(already has taken the page lock because the 
> preceding jffs2_write_begin)
>  jffs2_write_inode_range
>   jffs2_reserve_space
>    mutex_lock(&c->alloc_sem) A

I looked at the code, and you are right: When swapping 
jffs2_reserve_space() and grab_cache_page_write_begin() in my patch, this 
changed the lock acquisition order from lock_page / c->alloc_sem / f->sem 
to c->alloc_sem / f->sem / lock_page. [The intention was to acquire f->sem 
before lock_page, but c->alloc_sem must be acquired before f->sem 
according to README.Locking.]

However, I am surprised that jffs2_write_end() may run, for the same page, 
in parallel with jffs2_write_begin(); I would expect that it cannot be 
called before jffs2_write_begin() returns. Did your kernel actually panic 
on this? Do you have any further information about it?

Best regards,
Thomas