* [PATCH RESEND] jffs2: Fix lock acquisition order bug in jffs2_write_begin [not found] <507321E6.1050609@freenet.de> @ 2012-10-09 19:11 ` Thomas Betker 2012-10-10 6:58 ` Joakim Tjernlund 2012-10-17 12:42 ` Artem Bityutskiy 0 siblings, 2 replies; 4+ messages in thread From: Thomas Betker @ 2012-10-09 19:11 UTC (permalink / raw) To: linux-mtd, thomas.betker jffs2_write_begin() first acquires the page lock, then f->sem. This causes an AB-BA deadlock with jffs2_garbage_collect_live(), which first acquires f->sem, then the page lock: 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) We fix this by restructuring jffs2_write_begin() to take f->sem before the page lock. However, we make sure that f->sem is not held when calling jffs2_reserve_space(), as this is not permitted by the locking rules. The deadlock above was observed multiple times on an SoC with a dual ARMv7 (Cortex-A9), running the long-term 3.4.11 kernel; it occurred when using scp to copy files from a host system to the ARM target system. The fix was heavily tested on the same target system. If the patch is accepted, please get it also pushed to 3.4; it applies cleanly both to linux-mtd.git and the current linux-3.4 tree. Cc: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> Signed-off-by: Thomas Betker <thomas.betker@rohde-schwarz.com> diff -Naur linux-3.4.11-orig/fs/jffs2/file.c linux-3.4.11/fs/jffs2/file.c --- linux-3.4.11-orig/fs/jffs2/file.c 2012-09-14 22:18:55.000000000 +0000 +++ linux-3.4.11/fs/jffs2/file.c 2012-10-05 09:01:10.000000000 +0000 @@ -138,33 +138,39 @@ struct page *pg; struct inode *inode = mapping->host; struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode); + struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); + struct jffs2_raw_inode ri; + uint32_t alloc_len = 0; pgoff_t index = pos >> PAGE_CACHE_SHIFT; uint32_t pageofs = index << PAGE_CACHE_SHIFT; int ret = 0; + jffs2_dbg(1, "%s()\n", __func__); + + if (pageofs > inode->i_size) { + ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len, + ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE); + if (ret) + return ret; + } + + mutex_lock(&f->sem); pg = grab_cache_page_write_begin(mapping, index, flags); - if (!pg) + if (!pg) { + if (alloc_len) + jffs2_complete_reservation(c); + mutex_unlock(&f->sem); return -ENOMEM; + } *pagep = pg; - jffs2_dbg(1, "%s()\n", __func__); - - if (pageofs > inode->i_size) { + if (alloc_len) { /* Make new hole frag from old EOF to new page */ - struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb); - struct jffs2_raw_inode ri; struct jffs2_full_dnode *fn; - uint32_t alloc_len; jffs2_dbg(1, "Writing new hole frag 0x%x-0x%x between current EOF and new page\n", (unsigned int)inode->i_size, pageofs); - ret = jffs2_reserve_space(c, sizeof(ri), &alloc_len, - ALLOC_NORMAL, JFFS2_SUMMARY_INODE_SIZE); - if (ret) - goto out_page; - - mutex_lock(&f->sem); memset(&ri, 0, sizeof(ri)); ri.magic = cpu_to_je16(JFFS2_MAGIC_BITMASK); @@ -191,7 +197,6 @@ if (IS_ERR(fn)) { ret = PTR_ERR(fn); jffs2_complete_reservation(c); - mutex_unlock(&f->sem); goto out_page; } ret = jffs2_add_full_dnode_to_inode(c, f, fn); @@ -206,12 +211,10 @@ jffs2_mark_node_obsolete(c, fn->raw); jffs2_free_full_dnode(fn); jffs2_complete_reservation(c); - mutex_unlock(&f->sem); goto out_page; } jffs2_complete_reservation(c); inode->i_size = pageofs; - mutex_unlock(&f->sem); } /* @@ -220,18 +223,18 @@ * case of a short-copy. */ if (!PageUptodate(pg)) { - mutex_lock(&f->sem); ret = jffs2_do_readpage_nolock(inode, pg); - mutex_unlock(&f->sem); if (ret) goto out_page; } + mutex_unlock(&f->sem); jffs2_dbg(1, "end write_begin(). pg->flags %lx\n", pg->flags); return ret; out_page: unlock_page(pg); page_cache_release(pg); + mutex_unlock(&f->sem); return ret; } ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND] jffs2: Fix lock acquisition order bug in jffs2_write_begin 2012-10-09 19:11 ` [PATCH RESEND] jffs2: Fix lock acquisition order bug in jffs2_write_begin Thomas Betker @ 2012-10-10 6:58 ` Joakim Tjernlund 2012-10-17 12:42 ` Artem Bityutskiy 1 sibling, 0 replies; 4+ messages in thread From: Joakim Tjernlund @ 2012-10-10 6:58 UTC (permalink / raw) To: Thomas Betker; +Cc: linux-mtd, thomas.betker > > jffs2_write_begin() first acquires the page lock, then f->sem. This causes an AB-BA deadlock with jffs2_garbage_collect_live(), which first acquires f->sem, then the page lock: > > 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) > > We fix this by restructuring jffs2_write_begin() to take f->sem before the page lock. However, we make sure that f->sem is not held when calling jffs2_reserve_space(), as this is not permitted by the locking rules. > > The deadlock above was observed multiple times on an SoC with a dual ARMv7 (Cortex-A9), running the long-term 3.4.11 kernel; it occurred when using scp to copy files from a host system to the ARM target system. The fix was heavily tested on the same target system. > > If the patch is accepted, please get it also pushed to 3.4; it applies cleanly both to linux-mtd.git and the current linux-3.4 tree. > > Cc: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> > Signed-off-by: Thomas Betker <thomas.betker@rohde-schwarz.com> Acked-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> David, are you happy with this? Jocke ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND] jffs2: Fix lock acquisition order bug in jffs2_write_begin 2012-10-09 19:11 ` [PATCH RESEND] jffs2: Fix lock acquisition order bug in jffs2_write_begin Thomas Betker 2012-10-10 6:58 ` Joakim Tjernlund @ 2012-10-17 12:42 ` Artem Bityutskiy 2012-10-17 21:22 ` Thomas Betker 1 sibling, 1 reply; 4+ messages in thread From: Artem Bityutskiy @ 2012-10-17 12:42 UTC (permalink / raw) To: Thomas Betker; +Cc: linux-mtd, thomas.betker [-- Attachment #1: Type: text/plain, Size: 733 bytes --] On Tue, 2012-10-09 at 21:11 +0200, Thomas Betker wrote: > jffs2_write_begin() first acquires the page lock, then f->sem. This causes an AB-BA deadlock with jffs2_garbage_collect_live(), which first acquires f->sem, then the page lock: This patch does not apply to l2-mtd.git, which is based on 3.7-rc1. $:~/git/l2-mtd$ git apply --check ~/tmp/thomas.mbox error: patch failed: fs/jffs2/file.c:138 error: fs/jffs2/file.c: patch does not apply Could you please wrap commit message lines to 80 characters when you re-send? Also, would you please add Cc: stable@vger.kernel.org [vX.Y+], where vX.Y is the starting version for which this patch is relevant, e.g., v3.0. Thanks! -- Best Regards, Artem Bityutskiy [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RESEND] jffs2: Fix lock acquisition order bug in jffs2_write_begin 2012-10-17 12:42 ` Artem Bityutskiy @ 2012-10-17 21:22 ` Thomas Betker 0 siblings, 0 replies; 4+ messages in thread From: Thomas Betker @ 2012-10-17 21:22 UTC (permalink / raw) To: dedekind1; +Cc: linux-mtd, thomas.betker Hello Artem, I have mailed theupdated patch to the list as "[PATCH v3] ...". > This patch does not apply to l2-mtd.git, which is based on 3.7-rc1. > > $:~/git/l2-mtd$ git apply --check ~/tmp/thomas.mbox > error: patch failed: fs/jffs2/file.c:138 > error: fs/jffs2/file.c: patch does not apply Fixed, using another mailer ... > Could you please wrap commit message lines to 80 characters when you > re-send? Fixed -- sorry, this should not have happened. > Also, would you please add Cc: stable@vger.kernel.org [vX.Y+], where > vX.Y is the starting version for which this patch is relevant, e.g., > v3.0. Fixed, using [v3.4+]. (The patch may also be relevant for earlier kernel versions, but I don't have the means to test them.) Thanks, Thomas ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-17 21:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <507321E6.1050609@freenet.de>
2012-10-09 19:11 ` [PATCH RESEND] jffs2: Fix lock acquisition order bug in jffs2_write_begin Thomas Betker
2012-10-10 6:58 ` Joakim Tjernlund
2012-10-17 12:42 ` Artem Bityutskiy
2012-10-17 21:22 ` Thomas Betker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).