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

[Deng 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
 
It seems hard to fix the dead lock which decribed in the patch above by only changing the lock order in JFFS2.
There is another idea to fix this.
First, let us take a look at the dead lock which the patch try to fix.
jffs2_garbage_collect_live 
 mutex_lock(&f->sem) (A) 
 jffs2_garbage_collect_dnode 
  jffs2_gc_fetch_page 
   read_cache_page_async 
    do_read_cache_page 
     lock_page(page) (B) 
jffs2_write_begin 
 grab_cache_page_write_begin 
  find_lock_page 
   lock_page(page) (B) 
 mutex_lock(&f->sem) (A) 

In jffs2_garbage_collect_live, we can change read_cache_page_async(page)to something like read_cache_page_async_trylock(page), it only takes the page lock when the lock is not being taken, otherwise just returns result to upper. Once jffs2_garbage_collect_past knows the lock contentions through the return value of jffs2_garbage_collect_live, it can just return and wait the next garbage collect operation to continue the jffs2_gc_fetch_page opeation, keeps making another try to accquire the page lock until the lock is available.
There is no read_cache_page_async_trylock function in kernel yet, to achieve the function is not hard, But we need to add it to linux/mm first :(.