From patchwork Mon Sep 7 20:37:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Wilcox X-Patchwork-Id: 11762205 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 272AF112E for ; Mon, 7 Sep 2020 20:37:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F3DD1218AC for ; Mon, 7 Sep 2020 20:37:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="aLqR1UN3" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729303AbgIGUhQ (ORCPT ); Mon, 7 Sep 2020 16:37:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43898 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729186AbgIGUhO (ORCPT ); Mon, 7 Sep 2020 16:37:14 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CD0CDC061575; Mon, 7 Sep 2020 13:37:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description; bh=/Q+KDxFmJ8kENzdxoq4aNK25y+6WSS4+srMvBAkO44Q=; b=aLqR1UN3EAnDm7gBHsto+PmiVr r7/dI55AVUsoGp5ICxLG1791PtouAD688tZ6ATGYVCKVxZ0Fts2OYphZfbZw3e9Fal9H8x8498PhG zNkq2hd4OK1W8NRgf8UsepPhQuzxhoPPttPpH031APgLXChNe9mV1a6Y1YP38E2OoyBv6+5WJjKza upay1DaTkbk1HU/PKY8ZYR9zl8za6HDcZFJwVHKiDUJCFbMYsKuGDlZsFke3GU2yrBE5cGprOCynf /XvRZUkug1jANTkthTZNVgMA7tUCmPOWUNZwo5OKP0ZGLSz2cVbJ1rRuKp65PF2QyF0p2J+eDyj8k bRKQCldg==; Received: from willy by casper.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFNsy-00013E-PS; Mon, 07 Sep 2020 20:37:08 +0000 From: "Matthew Wilcox (Oracle)" To: Christoph Hellwig , "Darrick J . Wong" Cc: "Matthew Wilcox (Oracle)" , linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: [PATCH 1/2] iomap: Clear page error before beginning a write Date: Mon, 7 Sep 2020 21:37:06 +0100 Message-Id: <20200907203707.3964-2-willy@infradead.org> X-Mailer: git-send-email 2.21.3 In-Reply-To: <20200907203707.3964-1-willy@infradead.org> References: <20200907203707.3964-1-willy@infradead.org> MIME-Version: 1.0 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org If we find a page in write_begin which is !Uptodate, we need to clear any error on the page before starting to read data into it. This matches how filemap_fault(), do_read_cache_page() and generic_file_buffered_read() handle PageError on !Uptodate pages. When calling iomap_set_range_uptodate() in __iomap_write_begin(), blocks were not being marked as uptodate. This was found with generic/127 and a specially modified kernel which would fail (some) readahead I/Os. The test read some bytes in a prior page which caused readahead to extend into page 0x34. There was a subsequent write to page 0x34, followed by a read to page 0x34. Because the blocks were still marked as !Uptodate, the read caused all blocks to be re-read, overwriting the write. With this change, and the next one, the bytes which were written are marked as being Uptodate, so even though the page is still marked as !Uptodate, the blocks containing the written data are not re-read from storage. Fixes: 9dc55f1389f9 ("iomap: add support for sub-pagesize buffered I/O without buffer heads") Signed-off-by: Matthew Wilcox (Oracle) Reviewed-by: Christoph Hellwig --- fs/iomap/buffered-io.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index bcfc288dba3f..c95454784df4 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -578,6 +578,7 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, if (PageUptodate(page)) return 0; + ClearPageError(page); do { iomap_adjust_read_range(inode, iop, &block_start, From patchwork Mon Sep 7 20:37:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Wilcox X-Patchwork-Id: 11762203 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E906592C for ; Mon, 7 Sep 2020 20:37:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CC1AE2177B for ; Mon, 7 Sep 2020 20:37:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="BP+h+jIe" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729269AbgIGUhP (ORCPT ); Mon, 7 Sep 2020 16:37:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43890 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729184AbgIGUhO (ORCPT ); Mon, 7 Sep 2020 16:37:14 -0400 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C5F53C061573; Mon, 7 Sep 2020 13:37:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description; bh=rgfnIrHnaAEI6RXa4m3uPfmsWUpeORTndps4hZE1Wow=; b=BP+h+jIeaZhfpoJYvQtJPTIn1g rP9UigoetxvDNdDyxvc0RcEkLodhk+1xHLgHQhX1UkgMAu+xiuMye/3xSUCKzL2F1qLGPHfdOaM7G 7X32qBBWaysEGewE797mvwFa5It54OilLN3WRBs+MYmzVbDMzVB4eND8tPb6KlIgE3YxesEVce6VT +BUsSaBH4+fLVM9CA45kWOB5YeOWJjdQsUtT924BDvBhnMBNNT0G9j3r7r/KJIieoQffKeTgp3/Lm zEVbJkuq9UXNdImvpKXUZRMIBPwT0CmUV2dwo7t4BGhuYgPOqd/Am/2ndMe0EXyr0jEbKGc51Mf0Q d2qkUBsQ==; Received: from willy by casper.infradead.org with local (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFNsz-00013I-0f; Mon, 07 Sep 2020 20:37:09 +0000 From: "Matthew Wilcox (Oracle)" To: Christoph Hellwig , "Darrick J . Wong" Cc: "Matthew Wilcox (Oracle)" , linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: [PATCH 2/2] iomap: Mark read blocks uptodate in write_begin Date: Mon, 7 Sep 2020 21:37:07 +0100 Message-Id: <20200907203707.3964-3-willy@infradead.org> X-Mailer: git-send-email 2.21.3 In-Reply-To: <20200907203707.3964-1-willy@infradead.org> References: <20200907203707.3964-1-willy@infradead.org> MIME-Version: 1.0 Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-xfs@vger.kernel.org When bringing (portions of) a page uptodate, we were marking blocks that were zeroed as being uptodate, but not blocks that were read from storage. Like the previous commit, this problem was found with generic/127 and a kernel which failed readahead I/Os. This bug causes writes to be silently lost when working with flaky storage. Fixes: 9dc55f1389f9 ("iomap: add support for sub-pagesize buffered I/O without buffer heads") Signed-off-by: Matthew Wilcox (Oracle) Reviewed-by: Christoph Hellwig --- fs/iomap/buffered-io.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index c95454784df4..897ab9a26a74 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -574,7 +574,6 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, loff_t block_start = pos & ~(block_size - 1); loff_t block_end = (pos + len + block_size - 1) & ~(block_size - 1); unsigned from = offset_in_page(pos), to = from + len, poff, plen; - int status; if (PageUptodate(page)) return 0; @@ -595,14 +594,13 @@ __iomap_write_begin(struct inode *inode, loff_t pos, unsigned len, int flags, if (WARN_ON_ONCE(flags & IOMAP_WRITE_F_UNSHARE)) return -EIO; zero_user_segments(page, poff, from, to, poff + plen); - iomap_set_range_uptodate(page, poff, plen); - continue; + } else { + int status = iomap_read_page_sync(block_start, page, + poff, plen, srcmap); + if (status) + return status; } - - status = iomap_read_page_sync(block_start, page, poff, plen, - srcmap); - if (status) - return status; + iomap_set_range_uptodate(page, poff, plen); } while ((block_start += plen) < block_end); return 0;