Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933847AbbHJKQG (ORCPT ); Mon, 10 Aug 2015 06:16:06 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:43874 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933804AbbHJKPv (ORCPT ); Mon, 10 Aug 2015 06:15:51 -0400 Content-Type: text/plain; charset="UTF-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 From: Ben Hutchings To: linux-kernel@vger.kernel.org, stable@vger.kernel.org CC: akpm@linux-foundation.org, "Theodore Ts'o" Date: Mon, 10 Aug 2015 12:12:30 +0200 Message-ID: X-Mailer: LinuxStableQueue (scripts by bwh) Subject: [PATCH 3.2 029/110] ext4: fix race between truncate and __ext4_journalled_writepage() In-Reply-To: X-SA-Exim-Connect-IP: 2.173.94.72 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4768 Lines: 137 3.2.71-rc1 review patch. If anyone has any objections, please let me know. ------------------ From: Theodore Ts'o commit bdf96838aea6a265f2ae6cbcfb12a778c84a0b8e upstream. The commit cf108bca465d: "ext4: Invert the locking order of page_lock and transaction start" caused __ext4_journalled_writepage() to drop the page lock before the page was written back, as part of changing the locking order to jbd2_journal_start -> page_lock. However, this introduced a potential race if there was a truncate racing with the data=journalled writeback mode. Fix this by grabbing the page lock after starting the journal handle, and then checking to see if page had gotten truncated out from under us. This fixes a number of different warnings or BUG_ON's when running xfstests generic/086 in data=journalled mode, including: jbd2_journal_dirty_metadata: vdc-8: bad jh for block 115643: transaction (ee3fe7 c0, 164), jh->b_transaction ( (null), 0), jh->b_next_transaction ( (null), 0), jlist 0 - and - kernel BUG at /usr/projects/linux/ext4/fs/jbd2/transaction.c:2200! ... Call Trace: [] ? __ext4_journalled_invalidatepage+0x117/0x117 [] __ext4_journalled_invalidatepage+0x10f/0x117 [] ? __ext4_journalled_invalidatepage+0x117/0x117 [] ? lock_buffer+0x36/0x36 [] ext4_journalled_invalidatepage+0xd/0x22 [] do_invalidatepage+0x22/0x26 [] truncate_inode_page+0x5b/0x85 [] truncate_inode_pages_range+0x156/0x38c [] truncate_inode_pages+0x11/0x15 [] truncate_pagecache+0x55/0x71 [] ext4_setattr+0x4a9/0x560 [] ? current_kernel_time+0x10/0x44 [] notify_change+0x1c7/0x2be [] do_truncate+0x65/0x85 [] ? file_ra_state_init+0x12/0x29 - and - WARNING: CPU: 1 PID: 1331 at /usr/projects/linux/ext4/fs/jbd2/transaction.c:1396 irty_metadata+0x14a/0x1ae() ... Call Trace: [] ? console_unlock+0x3a1/0x3ce [] dump_stack+0x48/0x60 [] warn_slowpath_common+0x89/0xa0 [] ? jbd2_journal_dirty_metadata+0x14a/0x1ae [] warn_slowpath_null+0x14/0x18 [] jbd2_journal_dirty_metadata+0x14a/0x1ae [] __ext4_handle_dirty_metadata+0xd4/0x19d [] write_end_fn+0x40/0x53 [] ext4_walk_page_buffers+0x4e/0x6a [] ext4_writepage+0x354/0x3b8 [] ? mpage_release_unused_pages+0xd4/0xd4 [] ? wait_on_buffer+0x2c/0x2c [] ? ext4_writepage+0x3b8/0x3b8 [] __writepage+0x10/0x2e [] write_cache_pages+0x22d/0x32c [] ? ext4_writepage+0x3b8/0x3b8 [] ext4_writepages+0x102/0x607 [] ? sched_clock_local+0x10/0x10e [] ? __lock_is_held+0x2e/0x44 [] ? lock_is_held+0x43/0x51 [] do_writepages+0x1c/0x29 [] __writeback_single_inode+0xc3/0x545 [] writeback_sb_inodes+0x21f/0x36d ... Signed-off-by: Theodore Ts'o [bwh: Backported to 3.2: adjust context] Signed-off-by: Ben Hutchings --- fs/ext4/inode.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -1849,18 +1849,31 @@ static int __ext4_journalled_writepage(s page_bufs = page_buffers(page); BUG_ON(!page_bufs); walk_page_buffers(handle, page_bufs, 0, len, NULL, bget_one); - /* As soon as we unlock the page, it can go away, but we have - * references to buffers so we are safe */ + /* + * We need to release the page lock before we start the + * journal, so grab a reference so the page won't disappear + * out from under us. + */ + get_page(page); unlock_page(page); handle = ext4_journal_start(inode, ext4_writepage_trans_blocks(inode)); if (IS_ERR(handle)) { ret = PTR_ERR(handle); - goto out; + put_page(page); + goto out_no_pagelock; } - BUG_ON(!ext4_handle_valid(handle)); + lock_page(page); + put_page(page); + if (page->mapping != mapping) { + /* The page got truncated from under us */ + ext4_journal_stop(handle); + ret = 0; + goto out; + } + ret = walk_page_buffers(handle, page_bufs, 0, len, NULL, do_journal_get_write_access); @@ -1876,6 +1889,8 @@ static int __ext4_journalled_writepage(s walk_page_buffers(handle, page_bufs, 0, len, NULL, bput_one); ext4_set_inode_state(inode, EXT4_STATE_JDATA); out: + unlock_page(page); +out_no_pagelock: return ret; } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/