From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id F206CE77188 for ; Fri, 10 Jan 2025 16:50:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7ACE86B00C8; Fri, 10 Jan 2025 11:50:23 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 7369C6B00C9; Fri, 10 Jan 2025 11:50:23 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5ADEC6B00CA; Fri, 10 Jan 2025 11:50:23 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 361CA6B00C8 for ; Fri, 10 Jan 2025 11:50:23 -0500 (EST) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id D53E61A0C34 for ; Fri, 10 Jan 2025 16:50:22 +0000 (UTC) X-FDA: 82992130284.04.B96D9C9 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf20.hostedemail.com (Postfix) with ESMTP id 217901C0009 for ; Fri, 10 Jan 2025 16:50:20 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=vHRV9nyq; spf=pass (imf20.hostedemail.com: domain of djwong@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=djwong@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1736527821; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=/93gIfNRCZiMxEmSj2TX6IXIp0P4kiXAW8ER/WjSBW4=; b=aOzKfpFgAIhXuHboJrtzF+ygPinWKfEmAD79c9AR370GrIfbmAr+XfBhojIyxwuoQ2JLpd NP4WBK48TUdPC9vayMaau8bWaegqV9Jpo1x2wtMg5AaCm3h2eyZC3CcvxdXv+zWEFTLktK gOGbr2OJmscjDMIO/FGVqa9HHyIdpPc= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=vHRV9nyq; spf=pass (imf20.hostedemail.com: domain of djwong@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=djwong@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736527821; a=rsa-sha256; cv=none; b=X2OVpB97CMPFaJXXVJK2QPdtq4Kom530SMKxltHTVohcZ90yzbhEy/reOSWAuTHWal2YEJ BGcGKxIOI1RGCuRAdP9CsCAFbJ8RtT6rqMRzqW+CxbkFVuVKDxKEbjfMvcBJJJCwSnuTz7 BboFkvRN0oGsTpbBcm718d3HOnzbDcg= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 6AA7B5C1109; Fri, 10 Jan 2025 16:49:39 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 09E81C4CED6; Fri, 10 Jan 2025 16:50:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736527820; bh=5VSUGSu9BR1n/nKzKdICXzahPQ4soShf72WzqIUuGnE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=vHRV9nyqpL+Ro7f3yAiw9q++PTaPDuF/ivKV9lmJ4Ukmz/yQRbrjnqsoGlttY/XBE Hq9zsm1IRvDBUJnmkp4CMx5ZoJwmszsP87Mt6ywM6PqXIwvlKTF6CPT/tknjZuZk7j w8Vnd87saIzejPxlLTyqpOGOqOhVoMYtwGiWX5Qe3IUJCaT066yCqLUUrgRR5rdbMj ty7WZb3SdAG0Y0EjvRRAXWvRACRC9iMC0RVfZTAL3YY9+yQVa2QyyJhQn6G9vgxX4P CiyXkEafzE/5RH7p1+fdiZxkwvvPC6ygb5InHSPPT0iJ2/hXirT2MXnGJ0/qT2Axab jaN+omHLMWPwQ== Date: Fri, 10 Jan 2025 08:50:19 -0800 From: "Darrick J. Wong" To: Alistair Popple Cc: akpm@linux-foundation.org, dan.j.williams@intel.com, linux-mm@kvack.org, alison.schofield@intel.com, lina@asahilina.net, zhang.lyra@gmail.com, gerald.schaefer@linux.ibm.com, vishal.l.verma@intel.com, dave.jiang@intel.com, logang@deltatee.com, bhelgaas@google.com, jack@suse.cz, jgg@ziepe.ca, catalin.marinas@arm.com, will@kernel.org, mpe@ellerman.id.au, npiggin@gmail.com, dave.hansen@linux.intel.com, ira.weiny@intel.com, willy@infradead.org, tytso@mit.edu, linmiaohe@huawei.com, david@redhat.com, peterx@redhat.com, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linuxppc-dev@lists.ozlabs.org, nvdimm@lists.linux.dev, linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, jhubbard@nvidia.com, hch@lst.de, david@fromorbit.com, chenhuacai@kernel.org, kernel@xen0n.name, loongarch@lists.linux.dev Subject: Re: [PATCH v6 07/26] fs/dax: Ensure all pages are idle prior to filesystem unmount Message-ID: <20250110165019.GK6156@frogsfrogsfrogs> References: <704662ae360abeb777ed00efc6f8f232a79ae4ff.1736488799.git-series.apopple@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <704662ae360abeb777ed00efc6f8f232a79ae4ff.1736488799.git-series.apopple@nvidia.com> X-Rspamd-Queue-Id: 217901C0009 X-Stat-Signature: raqedjsm54y8ki35p9gabmu1s686gdme X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1736527820-186941 X-HE-Meta: U2FsdGVkX192KqgwfjahBppr90r0fYnkn/V17tRfqoJGSzVCNtSYcAPxUNKI6BLePmKitBKzv3BtghE2zVJbKdRJCGDu/jd0JyTnAyXjs4e5pReHfi5KIXNULCsAV66+4LEtfRcOb++Kqr8oJlLFxven4Mv3V7s/C/y9MR4dkk0dwqCCAT+9Hij3Zozm5oghSSLTw4I4jQOqs+EiwGlUvcMUS0Ew+PS408StVrfcw3mTRI6iGIm4YxlzK+Kp1LMehXlokUeIVf9ykL70sBk1OiiZgWVFv1RregatNdLIy1tl5s/yfIR2+62OgP66GKldRNJbLncPAq85udhijUq7D8+MlKZIS++lkbBqD0i1vC5CWPIDXJ2kMHJ/IHufh0LYDwazJd2NIBX3R5bYlcoU8zM7hGpUyxbnunxGi0UKDw/G4pATGXKmPis4nSNqSQDWWaQlL5AyP0DfksRvjQinbzLuJJxWIeyWX27IOpdVgqsawcQQyjngrYueR/SBRXI5sBSMXwDSKlycvpdABCteZlMpbiNPHpxkwouKLrl7ytkvulY/waBb1pI2yQ1N+XH416vrJtdc6hkURxedO3wCqVDbplgOL/W9tc/ZRY9/08vtqspMvAIU8iZDZgxZWCxrYG1TSWTikKUedbrUJTXNrG+pVDcrhc8Nqo7XdklQ4vAMbbgZPttgfFdwT61DnSw9jkpjPA13rxpbE52+aCKS3xoPDGEr+sCFKMKBTIklZIfIIDFu7+eDdAxnvxbYugITzvTR5QSQAei/5UBAZm5jq5rFL2tj8UYcEenazHC91I7QW9EXEg3Yurlaf2iH4KYDbA8QUC56Y2kITKQFvYhEoThx6GADhFX6s1DwzQ3WA2qKoxMrBRR+n9s9iPLo4r8FVgcmqdjWWuj6/O7rBl3JQ7Z5OjrZ1L98xcJDx/URK1WhFZ3kBVWNDNRP31vq4oZM49b9pc2IryWLXA+v/cM 7WH0rAxh QmjqrD6OUZ5X2LLcEd3B5e1Y8EH51fHdLa0Mu8sQRunixh4cchlLevZ0Dg5WyfRIEllGAQQn0alsQFNlyKL/vhjNzRYZJg7YZLbLFLkkBIFP+O/FooUNZoqiKDqG8I5nsbwFfu7joRRy997B3gkH/sstWg6JKSlVasHiUF7RI3tIh0YKjdxN5IDPLuW56fD7El5t9R/sJCRvEQLed3CR270jM1wo+WLPu7BRES1mkjC/DbpGDG8AjJ0N2UnTY/8U3IiK3Wq78NRKTfXaYaYfthyJiJ1crrVzGLLQvXVoyjjh5b3j8r/8FY0SiKEHmB4NAYyD5WJwZr47koamJqvAhJ9Yfyybk0xeRRTs9 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Jan 10, 2025 at 05:00:35PM +1100, Alistair Popple wrote: > File systems call dax_break_mapping() prior to reallocating file > system blocks to ensure the page is not undergoing any DMA or other > accesses. Generally this is needed when a file is truncated to ensure > that if a block is reallocated nothing is writing to it. However > filesystems currently don't call this when an FS DAX inode is evicted. > > This can cause problems when the file system is unmounted as a page > can continue to be under going DMA or other remote access after > unmount. This means if the file system is remounted any truncate or > other operation which requires the underlying file system block to be > freed will not wait for the remote access to complete. Therefore a > busy block may be reallocated to a new file leading to corruption. > > Signed-off-by: Alistair Popple > > --- > > Changes for v5: > > - Don't wait for pages to be idle in non-DAX mappings > --- > fs/dax.c | 29 +++++++++++++++++++++++++++++ > fs/ext4/inode.c | 32 ++++++++++++++------------------ > fs/xfs/xfs_inode.c | 9 +++++++++ > fs/xfs/xfs_inode.h | 1 + > fs/xfs/xfs_super.c | 18 ++++++++++++++++++ > include/linux/dax.h | 2 ++ > 6 files changed, 73 insertions(+), 18 deletions(-) > > diff --git a/fs/dax.c b/fs/dax.c > index 7008a73..4e49cc4 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -883,6 +883,14 @@ static int wait_page_idle(struct page *page, > TASK_INTERRUPTIBLE, 0, 0, cb(inode)); > } > > +static void wait_page_idle_uninterruptible(struct page *page, > + void (cb)(struct inode *), > + struct inode *inode) > +{ > + ___wait_var_event(page, page_ref_count(page) == 1, > + TASK_UNINTERRUPTIBLE, 0, 0, cb(inode)); > +} > + > /* > * Unmaps the inode and waits for any DMA to complete prior to deleting the > * DAX mapping entries for the range. > @@ -911,6 +919,27 @@ int dax_break_mapping(struct inode *inode, loff_t start, loff_t end, > } > EXPORT_SYMBOL_GPL(dax_break_mapping); > > +void dax_break_mapping_uninterruptible(struct inode *inode, > + void (cb)(struct inode *)) > +{ > + struct page *page; > + > + if (!dax_mapping(inode->i_mapping)) > + return; > + > + do { > + page = dax_layout_busy_page_range(inode->i_mapping, 0, > + LLONG_MAX); > + if (!page) > + break; > + > + wait_page_idle_uninterruptible(page, cb, inode); > + } while (true); > + > + dax_delete_mapping_range(inode->i_mapping, 0, LLONG_MAX); > +} > +EXPORT_SYMBOL_GPL(dax_break_mapping_uninterruptible); > + > /* > * Invalidate DAX entry if it is clean. > */ > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index ee8e83f..fa35161 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -163,6 +163,18 @@ int ext4_inode_is_fast_symlink(struct inode *inode) > (inode->i_size < EXT4_N_BLOCKS * 4); > } > > +static void ext4_wait_dax_page(struct inode *inode) > +{ > + filemap_invalidate_unlock(inode->i_mapping); > + schedule(); > + filemap_invalidate_lock(inode->i_mapping); > +} > + > +int ext4_break_layouts(struct inode *inode) > +{ > + return dax_break_mapping_inode(inode, ext4_wait_dax_page); > +} > + > /* > * Called at the last iput() if i_nlink is zero. > */ > @@ -181,6 +193,8 @@ void ext4_evict_inode(struct inode *inode) > > trace_ext4_evict_inode(inode); > > + dax_break_mapping_uninterruptible(inode, ext4_wait_dax_page); > + > if (EXT4_I(inode)->i_flags & EXT4_EA_INODE_FL) > ext4_evict_ea_inode(inode); > if (inode->i_nlink) { > @@ -3902,24 +3916,6 @@ int ext4_update_disksize_before_punch(struct inode *inode, loff_t offset, > return ret; > } > > -static void ext4_wait_dax_page(struct inode *inode) > -{ > - filemap_invalidate_unlock(inode->i_mapping); > - schedule(); > - filemap_invalidate_lock(inode->i_mapping); > -} > - > -int ext4_break_layouts(struct inode *inode) > -{ > - struct page *page; > - int error; > - > - if (WARN_ON_ONCE(!rwsem_is_locked(&inode->i_mapping->invalidate_lock))) > - return -EINVAL; > - > - return dax_break_mapping_inode(inode, ext4_wait_dax_page); > -} > - > /* > * ext4_punch_hole: punches a hole in a file by releasing the blocks > * associated with the given offset and length > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c > index 4410b42..c7ec5ab 100644 > --- a/fs/xfs/xfs_inode.c > +++ b/fs/xfs/xfs_inode.c > @@ -2997,6 +2997,15 @@ xfs_break_dax_layouts( > return dax_break_mapping_inode(inode, xfs_wait_dax_page); > } > > +void > +xfs_break_dax_layouts_uninterruptible( > + struct inode *inode) > +{ > + xfs_assert_ilocked(XFS_I(inode), XFS_MMAPLOCK_EXCL); > + > + dax_break_mapping_uninterruptible(inode, xfs_wait_dax_page); > +} > + > int > xfs_break_layouts( > struct inode *inode, > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h > index c4f03f6..613797a 100644 > --- a/fs/xfs/xfs_inode.h > +++ b/fs/xfs/xfs_inode.h > @@ -594,6 +594,7 @@ xfs_itruncate_extents( > } > > int xfs_break_dax_layouts(struct inode *inode); > +void xfs_break_dax_layouts_uninterruptible(struct inode *inode); > int xfs_break_layouts(struct inode *inode, uint *iolock, > enum layout_break_reason reason); > > diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c > index 8524b9d..73ec060 100644 > --- a/fs/xfs/xfs_super.c > +++ b/fs/xfs/xfs_super.c > @@ -751,6 +751,23 @@ xfs_fs_drop_inode( > return generic_drop_inode(inode); > } > > +STATIC void > +xfs_fs_evict_inode( > + struct inode *inode) > +{ > + struct xfs_inode *ip = XFS_I(inode); > + uint iolock = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL; > + > + if (IS_DAX(inode)) { > + xfs_ilock(ip, iolock); > + xfs_break_dax_layouts_uninterruptible(inode); > + xfs_iunlock(ip, iolock); If we're evicting the inode, why is it necessary to take i_rwsem and the mmap invalidation lock? Shouldn't the evicting thread be the only one with access to this inode? --D > + } > + > + truncate_inode_pages_final(&inode->i_data); > + clear_inode(inode); > +} > + > static void > xfs_mount_free( > struct xfs_mount *mp) > @@ -1189,6 +1206,7 @@ static const struct super_operations xfs_super_operations = { > .destroy_inode = xfs_fs_destroy_inode, > .dirty_inode = xfs_fs_dirty_inode, > .drop_inode = xfs_fs_drop_inode, > + .evict_inode = xfs_fs_evict_inode, > .put_super = xfs_fs_put_super, > .sync_fs = xfs_fs_sync_fs, > .freeze_fs = xfs_fs_freeze, > diff --git a/include/linux/dax.h b/include/linux/dax.h > index ef9e02c..7c3773f 100644 > --- a/include/linux/dax.h > +++ b/include/linux/dax.h > @@ -274,6 +274,8 @@ static inline int __must_check dax_break_mapping_inode(struct inode *inode, > { > return dax_break_mapping(inode, 0, LLONG_MAX, cb); > } > +void dax_break_mapping_uninterruptible(struct inode *inode, > + void (cb)(struct inode *)); > int dax_dedupe_file_range_compare(struct inode *src, loff_t srcoff, > struct inode *dest, loff_t destoff, > loff_t len, bool *is_same, > -- > git-series 0.9.1 >