From patchwork Mon Nov 5 16:36:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Brian Foster X-Patchwork-Id: 10668605 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A0FB61709 for ; Mon, 5 Nov 2018 16:36:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9105229C14 for ; Mon, 5 Nov 2018 16:36:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 850CA29C42; Mon, 5 Nov 2018 16:36:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_NONE autolearn=unavailable version=3.3.1 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1BE2129C14 for ; Mon, 5 Nov 2018 16:36:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EE5106B0003; Mon, 5 Nov 2018 11:36:16 -0500 (EST) Delivered-To: linux-mm-outgoing@kvack.org Received: by kanga.kvack.org (Postfix, from userid 40) id E954A6B0006; Mon, 5 Nov 2018 11:36:16 -0500 (EST) X-Original-To: int-list-linux-mm@kvack.org X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D85526B0007; Mon, 5 Nov 2018 11:36:16 -0500 (EST) X-Original-To: linux-mm@kvack.org X-Delivered-To: linux-mm@kvack.org Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by kanga.kvack.org (Postfix) with ESMTP id AC6736B0003 for ; Mon, 5 Nov 2018 11:36:16 -0500 (EST) Received: by mail-qk1-f197.google.com with SMTP id s19so22587010qke.20 for ; Mon, 05 Nov 2018 08:36:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-original-authentication-results:x-gm-message-state:from:to:cc :subject:date:message-id; bh=N6E92zVoLYL2MHydS90TxemDe/HA4H+ZPUbXhbRsi84=; b=Eqm7exvIOAi7eyMrrfMGaSzk9FP7Fu04eQAxR1jp3cLix+oEJAArFqHKNiyg2uzK97 x/TKVihxhYnb7NG8n/Pa4ibgCtHBMuqYZwssqnwwYMXoOE319BVP7eljlHVIC34+pROY OBsvDvoTDmo79XDDAiVw5A+nYqt/syCZ6rHE/bKzAWrgMwXAtWPwyn/XXCwXl+y2REqK Bj7f+Bb4pa348SQPwKzQLA5m3uxb2G/F7TtmJL9X8hAWHqtNwPhEGt5c8m+BggBPvbnI wHWe/uqN8YbOhlor7X7apsYp2KGbOVgMGiWzkrt1fZDw7Gw2TcsKRex4N9GVTp9Bse/T BPvQ== X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of bfoster@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com X-Gm-Message-State: AGRZ1gK2eSXcRpItk135FbtYc/GRsGin+qtLpjaTZ8eTraBleLoyYT7z u8QrOTwOzVCTm+jbA9g2euOqxHkpvewN/R7LwwCvJ7zVsHktE83zwY5u3ohm5jFQNfAxbA9s4bL 1h28nHwe8iQLGwItS84mJ0ZIetg4gGO4x60pTSE7/72J1DTyAJ/TzW/Jk3ZHOEZT7Ww== X-Received: by 2002:a37:cc88:: with SMTP id n8mr20559092qkl.319.1541435776453; Mon, 05 Nov 2018 08:36:16 -0800 (PST) X-Google-Smtp-Source: AJdET5cKEsSKA88nXjD8olJOrDnCp+hkRAk2OguFBAJPjViEyrIfn6HysQRoMjU2FL/H8+tpdNR0 X-Received: by 2002:a37:cc88:: with SMTP id n8mr20559056qkl.319.1541435775831; Mon, 05 Nov 2018 08:36:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1541435775; cv=none; d=google.com; s=arc-20160816; b=MQHUIaGsu5r4l1QMzpK0LR7GLEOJiiloIcwPyVaMK/RVsdzB1uueTqHdURFi/TEGgI IaeXuXTkPfL/Xoimg2z+6fV5sOsaF6cNkWevD8C46zU6pRsnfD7eTOw0Wk+mU3lLfnqi KuekiMUiSPARWCq0MbtKBuMGyLpNoiUtZRA0q3mgfz/A/Ce6njchdCdprApCFPpAMWKR bLO9+WhM44OHXWdcp5U1gBXCjuex9cgPhSmr3hl1upcFF8anRH6gCvDtLswNS+UVMd5E FSntjRGuZ8id0/S3kHHyAK+1RRk0mRDeixhw8UrL2KIwStydGtWSbAhPLwv37CFqcPAE QIYg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=message-id:date:subject:cc:to:from; bh=N6E92zVoLYL2MHydS90TxemDe/HA4H+ZPUbXhbRsi84=; b=Pv7mPjNh+If28hRm2JdjwFUD2tbpwywPzwbaUctlHWErZ9XOqVv2weNSjN7llQONIY peNYBWZpXz+CTG0aW3n1QqqdSyw2nXOZmjWpvaFAkhy9noAXR0cZ1q4zTyr1IocjFyFA pfA1TMlfmz1yukxFUs5HD8CSipkcbNr6AmU41KUexi+hJRPPqf4evnzzEZ/FEGbC+hE6 7TTrYLkrPCXSw7Jkph3SDqIVfYfSrb2Z2tnjpBb6lcGBIlUhmUEtE5SWu1Iv94jitxWn dWMEcF0MIy/O0bPhwfie74Wse8e1WhU4Fkk3HhYHdN5NmwhZOz9eV2U4Mbz3SnTm+ZkF IdfA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of bfoster@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from mx1.redhat.com (mx1.redhat.com. [209.132.183.28]) by mx.google.com with ESMTPS id z137si299534qka.25.2018.11.05.08.36.15 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 05 Nov 2018 08:36:15 -0800 (PST) Received-SPF: pass (google.com: domain of bfoster@redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; Authentication-Results: mx.google.com; spf=pass (google.com: domain of bfoster@redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CEB68C04B2D1; Mon, 5 Nov 2018 16:36:14 +0000 (UTC) Received: from bfoster.bos.redhat.com (dhcp-41-2.bos.redhat.com [10.18.41.2]) by smtp.corp.redhat.com (Postfix) with ESMTP id E626B66D4A; Mon, 5 Nov 2018 16:36:13 +0000 (UTC) From: Brian Foster To: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org Cc: Dave Chinner Subject: [PATCH] mm: don't break integrity writeback on ->writepage() error Date: Mon, 5 Nov 2018 11:36:13 -0500 Message-Id: <20181105163613.7542-1-bfoster@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 05 Nov 2018 16:36:14 +0000 (UTC) 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: X-Virus-Scanned: ClamAV using ClamSMTP write_cache_pages() currently breaks out of the writepage loop in the event of a ->writepage() error. This causes problems for integrity writeback on XFS in the event of a persistent error as XFS expects to process every dirty+delalloc page such that it can discard delalloc blocks when real block allocation fails. Failure to handle all delalloc pages leaves the filesystem in an inconsistent state if the integrity writeback happens to be due to an unmount, for example. Update write_cache_pages() to continue processing pages for integrity writeback regardless of ->writepage() errors. Save the first encountered error and return it once complete. This facilitates XFS or any other fs that expects integrity writeback to process the entire set of dirty pages regardless of errors. Background writeback continues to exit on the first error encountered. Signed-off-by: Brian Foster --- Hi all, This was actually first posted[1] as a patch in XFS to not return errors from ->writepage() when called via write_cache_pages(). After some discussion with Dave, it was suggested that this is a write_cache_pages() bug rather than one in XFS. I think that could go either way, so I'm floating this patch as an alternative. FWIW, that same thread also includes a supporting patch for an fstests test[2] that demonstrates the original problem this patch attempts to resolve. This applies on top of v4.19 and I've tested it against XFS and ext4 (defaults) and not seen any regressions. Note that it's not clear to me if ext4 is affected by the same or similar problem and I skipped btrfs since it seems to duplicate all of the associated writeback code. Finally, I'm not totally sure about the ->for_sync bit in the error handling logic. I included it out of caution to try and handle any sort of potential (->sync_mode == WB_SYNC_NONE && ->for_sync == 1) combination, but that doesn't appear to be used anywhere that I can see. Instead, ->for_sync seems more like an exceptional case of ->sync_mode == WB_SYNC ALL. Thoughts? Brian [1] https://marc.info/?l=linux-xfs&m=154102085505264&w=2 [2] https://marc.info/?l=fstests&m=154031860022439&w=2 mm/page-writeback.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 84ae9bf5858a..9dbbf9465ff9 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2156,6 +2156,7 @@ int write_cache_pages(struct address_space *mapping, { int ret = 0; int done = 0; + int error; struct pagevec pvec; int nr_pages; pgoff_t uninitialized_var(writeback_index); @@ -2236,25 +2237,29 @@ int write_cache_pages(struct address_space *mapping, goto continue_unlock; trace_wbc_writepage(wbc, inode_to_bdi(mapping->host)); - ret = (*writepage)(page, wbc, data); - if (unlikely(ret)) { - if (ret == AOP_WRITEPAGE_ACTIVATE) { + error = (*writepage)(page, wbc, data); + if (unlikely(error)) { + if (error == AOP_WRITEPAGE_ACTIVATE) { unlock_page(page); - ret = 0; - } else { + error = 0; + } else if (wbc->sync_mode != WB_SYNC_ALL && + !wbc->for_sync) { /* - * done_index is set past this page, - * so media errors will not choke + * done_index is set past this page, so + * media errors will not choke * background writeout for the entire * file. This has consequences for * range_cyclic semantics (ie. it may * not be suitable for data integrity * writeout). */ + ret = error; done_index = page->index + 1; done = 1; break; } + if (!ret) + ret = error; } /*