Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/810310/?format=api
{ "id": 810310, "url": "http://patchwork.ozlabs.org/api/patches/810310/?format=api", "web_url": "http://patchwork.ozlabs.org/project/linux-ext4/patch/20170905223541.20594-7-ross.zwisler@linux.intel.com/", "project": { "id": 8, "url": "http://patchwork.ozlabs.org/api/projects/8/?format=api", "name": "Linux ext4 filesystem development", "link_name": "linux-ext4", "list_id": "linux-ext4.vger.kernel.org", "list_email": "linux-ext4@vger.kernel.org", "web_url": null, "scm_url": null, "webscm_url": null, "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<20170905223541.20594-7-ross.zwisler@linux.intel.com>", "list_archive_url": null, "date": "2017-09-05T22:35:38", "name": "[6/9] ext4: safely transition S_DAX on journaling changes", "commit_ref": null, "pull_url": null, "state": "new", "archived": true, "hash": "ebeee52cea92d33c2310bad1db1593afb56e05d7", "submitter": { "id": 46514, "url": "http://patchwork.ozlabs.org/api/people/46514/?format=api", "name": "Ross Zwisler", "email": "ross.zwisler@linux.intel.com" }, "delegate": null, "mbox": "http://patchwork.ozlabs.org/project/linux-ext4/patch/20170905223541.20594-7-ross.zwisler@linux.intel.com/mbox/", "series": [ { "id": 1660, "url": "http://patchwork.ozlabs.org/api/series/1660/?format=api", "web_url": "http://patchwork.ozlabs.org/project/linux-ext4/list/?series=1660", "date": "2017-09-05T22:35:36", "name": "add ext4 per-inode DAX flag", "version": 1, "mbox": "http://patchwork.ozlabs.org/series/1660/mbox/" } ], "comments": "http://patchwork.ozlabs.org/api/patches/810310/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/810310/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<linux-ext4-owner@vger.kernel.org>", "X-Original-To": "patchwork-incoming@ozlabs.org", "Delivered-To": "patchwork-incoming@ozlabs.org", "Authentication-Results": "ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-ext4-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)", "Received": [ "from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xn1pg5Ny3z9sP3\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed, 6 Sep 2017 08:38:51 +1000 (AEST)", "(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752716AbdIEWiU (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 5 Sep 2017 18:38:20 -0400", "from mga01.intel.com ([192.55.52.88]:4505 \"EHLO mga01.intel.com\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1754013AbdIEWgT (ORCPT <rfc822;linux-ext4@vger.kernel.org>);\n\tTue, 5 Sep 2017 18:36:19 -0400", "from fmsmga004.fm.intel.com ([10.253.24.48])\n\tby fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t05 Sep 2017 15:36:17 -0700", "from theros.lm.intel.com ([10.232.112.77])\n\tby fmsmga004.fm.intel.com with ESMTP; 05 Sep 2017 15:36:16 -0700" ], "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.41,481,1498546800\"; d=\"scan'208\";a=\"308314884\"", "From": "Ross Zwisler <ross.zwisler@linux.intel.com>", "To": "Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org", "Cc": "Ross Zwisler <ross.zwisler@linux.intel.com>,\n\t\"Darrick J. Wong\" <darrick.wong@oracle.com>,\n\t\"Theodore Ts'o\" <tytso@mit.edu>,\n\tAndreas Dilger <adilger.kernel@dilger.ca>,\n\tChristoph Hellwig <hch@lst.de>, Dan Williams <dan.j.williams@intel.com>,\n\tDave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.cz>,\n\tlinux-ext4@vger.kernel.org, linux-nvdimm@lists.01.org,\n\tlinux-xfs@vger.kernel.org, stable@vger.kernel.org", "Subject": "[PATCH 6/9] ext4: safely transition S_DAX on journaling changes", "Date": "Tue, 5 Sep 2017 16:35:38 -0600", "Message-Id": "<20170905223541.20594-7-ross.zwisler@linux.intel.com>", "X-Mailer": "git-send-email 2.9.5", "In-Reply-To": "<20170905223541.20594-1-ross.zwisler@linux.intel.com>", "References": "<20170905223541.20594-1-ross.zwisler@linux.intel.com>", "Sender": "linux-ext4-owner@vger.kernel.org", "Precedence": "bulk", "List-ID": "<linux-ext4.vger.kernel.org>", "X-Mailing-List": "linux-ext4@vger.kernel.org" }, "content": "The IOCTL path which switches the journaling mode for an inode is currently\nunsafe because it doesn't properly do a writeback and invalidation on the\ninode. In XFS, for example, safe transitions of S_DAX are handled by\nxfs_ioctl_setattr_dax_invalidate() which locks out page faults and I/O,\ndoes a writeback via filemap_write_and_wait() and an invalidation via\ninvalidate_inode_pages2().\n\nWithout this in place we can see the following kernel warning when we try\nand insert a DAX exceptional entry but find that a dirty page cache page is\nstill in the mapping->radix_tree:\n\n WARNING: CPU: 4 PID: 1052 at mm/filemap.c:262 __delete_from_page_cache+0x375/0x550\n Modules linked in: dax_pmem nd_pmem device_dax nd_btt nfit libnvdimm\n CPU: 4 PID: 1052 Comm: small Not tainted 4.13.0-rc6-00055-gac26931 #3\n Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.9.3-1.fc25 04/01/2014\n task: ffff88020ccd0000 task.stack: ffffc900021d4000\n RIP: 0010:__delete_from_page_cache+0x375/0x550\n RSP: 0000:ffffc900021d7b90 EFLAGS: 00010002\n RAX: 002fffc00001123d RBX: ffffffffffffffff RCX: ffff8801d9440d68\n RDX: 0000000000000000 RSI: ffffffff81fd5b84 RDI: ffffffff81f6f0e5\n RBP: ffffc900021d7be0 R08: 0000000000000000 R09: ffff8801f9938c70\n R10: 0000000000000021 R11: ffff8801f9938c91 R12: ffff8801d9440d70\n R13: ffffea0007fdda80 R14: 0000000000000001 R15: ffff8801d9440d68\n FS: 00007feacc041700(0000) GS:ffff880211800000(0000) knlGS:0000000000000000\n CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033\n CR2: 0000000010420000 CR3: 000000020cfd8000 CR4: 00000000000006e0\n Call Trace:\n dax_insert_mapping_entry+0x158/0x2c0\n dax_iomap_fault+0x1020/0x1bb0\n ext4_dax_huge_fault+0xc8/0x160\n ext4_dax_fault+0x10/0x20\n __do_fault+0x20/0x110\n __handle_mm_fault+0x97d/0x1120\n handle_mm_fault+0x188/0x2f0\n __do_page_fault+0x28f/0x590\n trace_do_page_fault+0x58/0x2c0\n do_async_page_fault+0x2c/0x90\n async_page_fault+0x28/0x30\n\nI'm pretty sure we could make a test that shows userspace visible data\ncorruption as well in this scenario.\n\nMake it safe to change the journaling mode and turn on or off S_DAX by\nadding locking to properly lock out page faults (i_mmap_sem) and then doing\nthe writeback and invalidate. I/O is already held off because all callers\nof ext4_ioctl_setflags() hold the inode lock.\n\nThe locking for this new code is complex because of the following:\n\n1) filemap_write_and_wait() eventually calls ext4_writepages(), which\nacquires the sbi->s_journal_flag_rwsem. This lock ranks above the\njbdw_handle which is eventually taken by ext4_journal_start(). This\nessentially means that the writeback has to happen outside of the context\nof an active journal handle (outside of ext4_journal_start() to\next4_journal_stop().)\n\n2) To lock out page faults we take a write lock on the ei->i_mmap_sem, and\nthis lock again ranks above the jbd2_handle taken by ext4_journal_start().\nSo, as with the writeback code in 1) above we have to take ei->i_mmap_sem\noutside of the context of an active journal handle.\n\nSigned-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>\nCC: stable@vger.kernel.org\n---\nPlease let me know if my \"context of an active journal handle\" terminology\nmakes sense, or if some other phrasing is more common.\n\nI'll work on an xfstest which shows this data corruption.\n---\n fs/ext4/inode.c | 25 +++++++++++++++----------\n 1 file changed, 15 insertions(+), 10 deletions(-)", "diff": "diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c\nindex d218991..facb5ae 100644\n--- a/fs/ext4/inode.c\n+++ b/fs/ext4/inode.c\n@@ -5949,13 +5949,15 @@ int ext4_change_inode_journal_flag(struct inode *inode, int val)\n \t * fallocate or buffered writes in dioread_nolock mode covered by\n \t * dirty data which can be converted only after flushing the dirty\n \t * data (and journalled aops don't know how to handle these cases).\n+\t *\n+\t * Because the changes to our journaling mode may also modify S_DAX we\n+\t * need to hold off I/O and page faults so we can write back any dirty\n+\t * data and invalidate all mappings.\n \t */\n-\tif (val) {\n-\t\tdown_write(&EXT4_I(inode)->i_mmap_sem);\n-\t\terr = filemap_write_and_wait(inode->i_mapping);\n-\t\tif (err < 0)\n-\t\t\tgoto out_unlock;\n-\t}\n+\tdown_write(&EXT4_I(inode)->i_mmap_sem);\n+\terr = filemap_write_and_wait(inode->i_mapping);\n+\tif (err < 0)\n+\t\tgoto out_unlock;\n \n \tpercpu_down_write(&sbi->s_journal_flag_rwsem);\n \tjbd2_journal_lock_updates(journal);\n@@ -5976,6 +5978,11 @@ int ext4_change_inode_journal_flag(struct inode *inode, int val)\n \t\t\tgoto out_journal_unlock;\n \t\text4_clear_inode_flag(inode, EXT4_INODE_JOURNAL_DATA);\n \t}\n+\n+\terr = invalidate_inode_pages2(inode->i_mapping);\n+\tif (err < 0)\n+\t\tgoto out_journal_unlock;\n+\n \text4_set_aops(inode);\n \t/*\n \t * Update inode->i_flags after EXT4_INODE_JOURNAL_DATA was updated.\n@@ -5986,8 +5993,7 @@ int ext4_change_inode_journal_flag(struct inode *inode, int val)\n \tjbd2_journal_unlock_updates(journal);\n \tpercpu_up_write(&sbi->s_journal_flag_rwsem);\n \n-\tif (val)\n-\t\tup_write(&EXT4_I(inode)->i_mmap_sem);\n+\tup_write(&EXT4_I(inode)->i_mmap_sem);\n \text4_inode_resume_unlocked_dio(inode);\n \n \t/* Finally we can mark the inode as dirty. */\n@@ -6007,8 +6013,7 @@ int ext4_change_inode_journal_flag(struct inode *inode, int val)\n \tjbd2_journal_unlock_updates(journal);\n \tpercpu_up_write(&sbi->s_journal_flag_rwsem);\n out_unlock:\n-\tif (val)\n-\t\tup_write(&EXT4_I(inode)->i_mmap_sem);\n+\tup_write(&EXT4_I(inode)->i_mmap_sem);\n \text4_inode_resume_unlocked_dio(inode);\n \treturn err;\n }\n", "prefixes": [ "6/9" ] }