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 DA96BC636D7 for ; Wed, 8 Feb 2023 17:12:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4B2636B0078; Wed, 8 Feb 2023 12:12:18 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 462486B007B; Wed, 8 Feb 2023 12:12:18 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 351DF6B007D; Wed, 8 Feb 2023 12:12:18 -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 26C1F6B0078 for ; Wed, 8 Feb 2023 12:12:18 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id E9C331A0E81 for ; Wed, 8 Feb 2023 17:12:17 +0000 (UTC) X-FDA: 80444767914.02.B9F5676 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf24.hostedemail.com (Postfix) with ESMTP id 30AA4180015 for ; Wed, 8 Feb 2023 17:12:13 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=KxWK1Ny4; spf=none (imf24.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1675876334; 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=XvevmZ7eJPj41qZI25XXutQP9fxV7mY1igR43Fu3cQg=; b=KBLGMaFMUCbol+LWpQDE+utB74//fbFalAqYHq4vLgnsXi3hhehW4mWAc0lO0C4yEm4NQh AGICo099ZAcof/k2PqqZUpsxmabegGkIivhmk0itpQeVgV+xXS9os+9xvhz3hjehorH/vo pfhsA8wB+yQxjMc8oQknUwJDHilU0kA= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=KxWK1Ny4; spf=none (imf24.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1675876334; a=rsa-sha256; cv=none; b=SuhXRCxj4Y0Rwcv5ndsi7HN4PmYVnHPBqjaXSPgRJKKJ3ypTq5XOSvyj7WIhpwEXgmO7Hs mahx97+Re9hfBmgeNqXC/4cpM63TTXpxvrX9ZS5AZJvHfMc88o3uKkjOpM7Qil2GmlYJ02 UErFwopwJ3AXtk3zTiwjI6rk1Z8/7JU= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=XvevmZ7eJPj41qZI25XXutQP9fxV7mY1igR43Fu3cQg=; b=KxWK1Ny4cSM7QoJ4hUOeMaPjYo BBFpA0mvKw5hnhRDLu+ii6DQjrTk+cVbsl8wsSdELGDixoTy8DcOCYvNVKLlM2bGIhLyiz+L4R+kY oWnpe1xMJuEfDNV9NpjF+68dnX884yNws82VILo5fYDed/KoY0/EqtMlO33wR5kaG150XrefhpnyX EL3GDPTu+cw58MKKbc5IIO8MXOEKKw1nF1p2JajsLRYbKvDuQ8f+S7PrjYk6H9SsFH6/91Y0wkivC 8W/CmSC2y+mv+wyK5IkTTNgCrmGVprhJEyHFjaCs878IltQ8HJCL8hj+90s+ROjHgav0Kag5KrwLq UN1VUXbw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pPnzK-001OGL-VM; Wed, 08 Feb 2023 17:12:07 +0000 Date: Wed, 8 Feb 2023 17:12:06 +0000 From: Matthew Wilcox To: "Darrick J. Wong" Cc: linux-xfs@vger.kernel.org, linux-afs@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, Dave Chinner Subject: Re: [PATCH 1/3] xfs: Remove xfs_filemap_map_pages() wrapper Message-ID: References: <20230208145335.307287-1-willy@infradead.org> <20230208145335.307287-2-willy@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 30AA4180015 X-Rspam-User: X-Stat-Signature: rs4qojn1ybsyi3o6di4hhyy8oycrbxoc X-HE-Tag: 1675876333-220853 X-HE-Meta: U2FsdGVkX198B+a3pUqpKkv0MhRUbAKWmJDFrvasriDhFudEVo5Qd9H1i+869wNVpmcmFPjyzgbZI+vkXAfXaeWS2XkAVHUyWcK7dRnKnMjRL9UccEwZmeDoHGl2haR+hlvQwbwQWIwU+h+QVGRhmxnO3cqEkTfG67J2SnXgm46Rm8roOXEVx322lOs7xJGzJoPQA8ZHnjuNGweUGmBLB9Xssy6euc5GEefNrJSMFiY1VNbskUYPe/UtYbV5lunBWtl/v4a8KcQP2w50JRf67S5wW/yvLdQbrvp12ppdnIbajvTXk6/hx9g4j4m8HznQIFfqVSnBrbcIU0dZwD07koorocRw2GTdFcpM4pJD8RPUBDJt4Csb+ug1lfDT3Kbmn1Bjr1PuE3pY4xVZRYu3ESkT+YcZcbhhKuNKl1O3nIGKnDNS0oEj+gXHyPVCOqVdma5p+RJ8EWtWmdOhB2y8TrSzxnD+mON0Fg/Oq4nmcC2VZnDErT30ihPLIEqTAnD5f7UYNY19BZSgOfJI3GhV1pO+Xoz/aHr8wIEEq8JBZSzCCFav5BXB3+Pf8zdxuKgvvC7Y9tnXkD+8WUJpqyMLAN0QyBBk/T4zEqoVpeCHu7RWtMtaoJlD2HNbngEyXzq1BWBoF/QTUFcp+hC5B4tfo0tPcD3UOsqPwISZdYpYsY08oLSlltCuCRD7gQtk4HzsLemDYuGHb1DwUIPR+QJLrLrIyuBc+lpX/t6pJHckr+yn5HYecEAB5ea+nnpXwSl3elL05PcfCy6AHONgwTOERYDXxEwb1kWLXonjxcb648+QT5rtOV8s7cyGbLOikrGRspRNZKBLhaUDwjO6WqIw1knbp3rAohoTojbr/mzuenSZjQ/3SP8aYnNPyUK2hboQ/K23itYbRY4wk+gsg65KWi4vKjXdYtxWX/CTYmyTpegTs7IZdTLw1nL2GbGjq+sfFPf4AyhpBA3Lw5sVw/f cQrJfcnN 4gobNfjanq+J/DJx20EWhmaYpYsiUsUZxPlL0HqFcf2TD/9fgBn+jgoxr3K25DbdCuV97eewmadLcgSJXWSCrkcStkj6C4znFADcY+3WMyDlK2xDAqY4HH6gMcREpnl2uFgdoB+7rkPRTdLbbvebEweQQQK1h2vm/81gcysChkg9Igvk2/Z5PBShcewTumwYJz6QgK4z6tLNaDso= 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: On Wed, Feb 08, 2023 at 08:39:19AM -0800, Darrick J. Wong wrote: > On Wed, Feb 08, 2023 at 02:53:33PM +0000, Matthew Wilcox (Oracle) wrote: > > XFS doesn't actually need to be holding the XFS_MMAPLOCK_SHARED > > to do this, any more than it needs the XFS_MMAPLOCK_SHARED for a > > read() that hits in the page cache. > > Hmm. From commit cd647d5651c0 ("xfs: use MMAPLOCK around > filemap_map_pages()"): > > The page faultround path ->map_pages is implemented in XFS via > filemap_map_pages(). This function checks that pages found in page > cache lookups have not raced with truncate based invalidation by > checking page->mapping is correct and page->index is within EOF. > > However, we've known for a long time that this is not sufficient to > protect against races with invalidations done by operations that do > not change EOF. e.g. hole punching and other fallocate() based > direct extent manipulations. The way we protect against these > races is we wrap the page fault operations in a XFS_MMAPLOCK_SHARED > lock so they serialise against fallocate and truncate before calling > into the filemap function that processes the fault. > > Do the same for XFS's ->map_pages implementation to close this > potential data corruption issue. > > How do we prevent faultaround from racing with fallocate and reflink > calls that operate below EOF? I don't understand the commit message. It'd be nice to have an example of what's insufficient about the protection. If XFS really needs it, it can trylock the semaphore and return 0 if it fails, falling back to the ->fault path. But I don't think XFS actually needs it. The ->map_pages path trylocks the folio, checks the folio->mapping, checks uptodate, then checks beyond EOF (not relevant to hole punch). Then it takes the page table lock and puts the page(s) into the page tables, unlocks the folio and moves on to the next folio. The hole-punch path, like the truncate path, takes the folio lock, unmaps the folio (which will take the page table lock) and removes it from the page cache. So what's the race?