From patchwork Tue May 23 08:13:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Hellwig X-Patchwork-Id: 13251837 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DE2C3C77B75 for ; Tue, 23 May 2023 08:16:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236112AbjEWIQG (ORCPT ); Tue, 23 May 2023 04:16:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38356 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235873AbjEWIPg (ORCPT ); Tue, 23 May 2023 04:15:36 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4D16010D0 for ; Tue, 23 May 2023 01:13:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; 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=7w/PRXKmGfEj1WmV8ZOxgi7sl9nXGx5xTQevApR0MrI=; b=QC7uShyYbkq7LuRBs41+kwITHX w20ax9hHI4yNSIP3ObTq1VP7yO/assr5y1Mej7DcPkheKeaIf51cdTHirclSGfhzhJZq/W55VJSx3 Ikrfa2D4ys8rS7qzYnhPCYb65Xpjrhk+66fg2Nf0SnV+Ms+qUAT3AlkFplewQkiJsAxk2lyDyeOy6 Ybo/0EAnEMkCZZRpEuHdoib1WOTkZx5yPz2fUCqKrMuRkBB+yYZ2oGBaqVpWoGdLrQAeaD0RCRzmu xErnNgz3Bs2hIgYwehfusJsjOBtQYHqoOz9TFyc1Iv5bU49AfckV0h07jt8skxMJj6wDNf7Cscatp 9oYAmUvQ==; Received: from [2001:4bb8:188:23b2:6ade:85c9:530f:6eb0] (helo=localhost) by bombadil.infradead.org with esmtpsa (Exim 4.96 #2 (Red Hat Linux)) id 1q1N9N-009OVF-0U; Tue, 23 May 2023 08:13:45 +0000 From: Christoph Hellwig To: Chris Mason , Josef Bacik , David Sterba Cc: linux-btrfs@vger.kernel.org Subject: [PATCH 07/16] btrfs: don't check PageError in __extent_writepage Date: Tue, 23 May 2023 10:13:13 +0200 Message-Id: <20230523081322.331337-8-hch@lst.de> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230523081322.331337-1-hch@lst.de> References: <20230523081322.331337-1-hch@lst.de> MIME-Version: 1.0 X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org __extent_writepage currenly sets PageError whenever any error happens, and the also checks for PageError to decide if to call error handling. This leads to very unclear responsibility for cleaning up on errors. In the VM and generic writeback helpers the basic idea is that once I/O is fired off all error handling responsibility is delegated to the end I/O handler. But if that end I/O handler sets the PageError bit, and the submitter checks it, the bit could in some cases leak into the submission context for fast enough I/O. Fix this by simply not checking PageError and just using the local ret variable to check for submission errors. This also fundamentally solves the long problem documented in a comment in __extent_writepage by never leaking the error bit into the submission context. Signed-off-by: Christoph Hellwig --- fs/btrfs/extent_io.c | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index b846c46c7a875b..d7b31888efa17a 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -1557,38 +1557,7 @@ static int __extent_writepage(struct page *page, struct btrfs_bio_ctrl *bio_ctrl set_page_writeback(page); end_page_writeback(page); } - /* - * Here we used to have a check for PageError() and then set @ret and - * call end_extent_writepage(). - * - * But in fact setting @ret here will cause different error paths - * between subpage and regular sectorsize. - * - * For regular page size, we never submit current page, but only add - * current page to current bio. - * The bio submission can only happen in next page. - * Thus if we hit the PageError() branch, @ret is already set to - * non-zero value and will not get updated for regular sectorsize. - * - * But for subpage case, it's possible we submit part of current page, - * thus can get PageError() set by submitted bio of the same page, - * while our @ret is still 0. - * - * So here we unify the behavior and don't set @ret. - * Error can still be properly passed to higher layer as page will - * be set error, here we just don't handle the IO failure. - * - * NOTE: This is just a hotfix for subpage. - * The root fix will be properly ending ordered extent when we hit - * an error during writeback. - * - * But that needs a bigger refactoring, as we not only need to grab the - * submitted OE, but also need to know exactly at which bytenr we hit - * the error. - * Currently the full page based __extent_writepage_io() is not - * capable of that. - */ - if (PageError(page)) + if (ret) end_extent_writepage(page, ret, page_start, page_end); if (bio_ctrl->extent_locked) { struct writeback_control *wbc = bio_ctrl->wbc;