From patchwork Thu Jan 25 08:57:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Hellwig X-Patchwork-Id: 13530274 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 44402C47422 for ; Thu, 25 Jan 2024 08:59:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CC3F628000D; Thu, 25 Jan 2024 03:59:17 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C753E280001; Thu, 25 Jan 2024 03:59:17 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AECB528000D; Thu, 25 Jan 2024 03:59:17 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 9C5B7280001 for ; Thu, 25 Jan 2024 03:59:17 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 69A6F1C1660 for ; Thu, 25 Jan 2024 08:59:17 +0000 (UTC) X-FDA: 81717234354.07.CB71E86 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf06.hostedemail.com (Postfix) with ESMTP id B9CB7180002 for ; Thu, 25 Jan 2024 08:59:15 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=yI1+JGkK; dmarc=none; spf=none (imf06.hostedemail.com: domain of BATV+b97a0fc658bf3e588113+7459+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+b97a0fc658bf3e588113+7459+infradead.org+hch@bombadil.srs.infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706173155; 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-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=W1Pg45e3YPveg08TzLy7LzAEvB67BYDtIDXVWrYOF4k=; b=UqUJXwgk0bnuR6A57varXnwKgpeGe/CKG6e7c3oR729NegzqXhGgL07zms/xRhPvmBQNZq vL5tZraD4z0gX+d0uvGz0KrucD7nfOZRWTc8BTnyLxANwGH53MPr0UyfVIMHbEKIDXXsHi P5lSndMVlY8G1nOXVOIVS1v/T98DlBg= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=yI1+JGkK; dmarc=none; spf=none (imf06.hostedemail.com: domain of BATV+b97a0fc658bf3e588113+7459+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+b97a0fc658bf3e588113+7459+infradead.org+hch@bombadil.srs.infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706173155; a=rsa-sha256; cv=none; b=uDdIFFDozpGplfu99nFtWVqBvTj/d/e+kaWNYnZg9LVtgCNn0iJPFKwhiN+Sy3jQI37VkQ QMjtDgxLMifN/QkRttDM23TM7/SZrpO+xHb7oWChNe0fNjjB8ndCXUoKobdQ8WOVrzCjJD X75rCHtnocrlji6lfi4z8uD+6IvhtVU= 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=W1Pg45e3YPveg08TzLy7LzAEvB67BYDtIDXVWrYOF4k=; b=yI1+JGkKxGm9WfeiG/b4Yka18x c1mXHaGCYgK6mkxGcVo5gAHbOKDx9+0SYzjipTUFCR7tVQHmx8jfKBwoUQSpzV0Hl0HSi6ZIxbk4X o+ZvB7bpE8nEto805bTnTxGxRhJsSbfgcVGM8+wIdOoEikiUBttNbyBcb1DNZ8s75wbzeVEW07tpZ qSvj6gzosLUHO646NIP3w95G/xf5aiufxBVQU+oWuTXercuNL6hOZVTSXRRdpOz7d754pCeI68r0r Fhvwk5s7Jrh9Acbu4JXBI1UWYk3FH9vxl0k/up76e5MCS4mN1NhPBCzaj0pMgfN7TGLtGYAFIzxdR l/0mO7bw==; Received: from 2a02-8389-2341-5b80-39d3-4735-9a3c-88d8.cable.dynamic.v6.surfer.at ([2a02:8389:2341:5b80:39d3:4735:9a3c:88d8] helo=localhost) by bombadil.infradead.org with esmtpsa (Exim 4.96 #2 (Red Hat Linux)) id 1rSvZq-007Qbq-19; Thu, 25 Jan 2024 08:59:14 +0000 From: Christoph Hellwig To: linux-mm@kvack.org Cc: Matthew Wilcox , Jan Kara , David Howells , Brian Foster , Christian Brauner , "Darrick J. Wong" , linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 19/19] writeback: simplify writeback iteration Date: Thu, 25 Jan 2024 09:57:58 +0100 Message-Id: <20240125085758.2393327-20-hch@lst.de> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240125085758.2393327-1-hch@lst.de> References: <20240125085758.2393327-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 X-Rspamd-Queue-Id: B9CB7180002 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: t1mp7d53bpzmnurr53mtf5k6hsndrpuf X-HE-Tag: 1706173155-290500 X-HE-Meta: U2FsdGVkX18thyNNuJwyojngFdGblC2t6+0emYWiRd+DiUWU/aAn0ObJDFOHo6nNkyaAZU0v7IC+p7hsdxs0HpepMtqi4RxD56UHUMgVdJQYUYWdqfuZBRxCtkvl8lfZPaKLCaySR9+yhGSQ3p1Eo4bQ+rykIrY2necKvAPyMAdjJtOi78Gzj/uVyTm5CW7z04uaCETD3QeiUS1Ihkc+dFKsZvWTQQydw+0pinj/sfTPit7iBfEwavsdH/kM8JSP0ojKN8kRYa0yXJrZfwJLK6J4vF6GMB49a7KdASmTau1KMDCelJ1aDDclZnfzE+FY8qjm4HtKB8cHloViSV//JIXBkPQtNjUIZj0/Td4n5ueDyZG5BiO9iJSyzY9ln1Oq7XAxUz6pChTRYp8FKRm+bJDfv3SIPnlD+RP0a8N8rLRoDtrKRCnZY9Ol7L9PkmF15QuzO6jIGQMHQoDC2JHxVkV3C/AZ/2/bSAP1QSn81FWbU/CKtF7wX6BPL05RzZhyeem26bfjBg+stlcHEX7ys9ZjyY2WlfYu6Z9A0oAuAb3dps0Oh3WOYb3yqtipR9JD9fhhY1eCf2GOWdKRaWld7ujeRFEY8Dr/9EvpoKlqJkDv0UMf+NC/+nua7DOcR/QdqGz3bHjqIRDgcmMa0DrKduuY8N6MfRnbau4hrv6poj5wZ8J3oPD/uPTpZosMxC4pWor1UOShaCzXoa1vOR/KAEGi7fFcRtu+jBjA0rsYDyZwBrA7TzESh4xWmfxrIUTdi/IuB+6VuryPZGSeJQq52aSVwBcEuLtppGHKK8Uv1JQX71rhNclT6nWZRj1isWIP8VoNkV8KSzGm+9RQJs7/hXkWWX9sWISsWbY+Bc57+Jqn9oQLJ8p/hp1TlIB3bNlpXjLA/Jmowj2NVa8SilbvjPhOwp6nxPofzsPzM+BrGcy/lzbr3Aa1XuIm8+v7ze3hv3xarJ+9lM485EG0cla skIPT8dH GpvqWyYJJMTeem9Qxwzu1zH9NpxeOZbeqoD+LGXwtBDj6TOj55fVIOa1iw4ZktVCZa3O0SeIdsPnielPKOrCNSoru+UbySJxxv8FV5Z8zGubPuc9Lu67WGSzQMspg5AvFTzodG/P00IbGFUHBi10lP5ZrocN6LiFvKB5k/3/4QPByYGDHAXGSdnyCmzGmUBeBG6GatgBCnldSlZI3ANuJsMEs8l6p0deBsGjB67fm2cOI/SQ0VSIjx5dVfJPCggXOMiYfEq8Hb5/ASiMp9rVruw+LG7BiTVZKcCFMdXJT8XFA7YM7IimYJxAfq8NQKEI6/Lv7y5S0H/ny+fFGxjxU85EcU9PJq5sfTMlF+nXrUN/PmBBu5rH5+szp6rWP2yUPcbMQu8w3cJQSNnWY5QnOC7rMFOF4Wg+SR99DC3KLpbm89z7DUgbJ/sRSXUO29AL7Lg3I 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: Based on the feedback from Jan I've tried to figure out how to avoid the error magic in the for_each_writeback_folio. This patch tries to implement this by switching to an open while loop over a single writeback_iter() function: while ((folio = writeback_iter(mapping, wbc, folio, &error))) { ... } the twist here is that the error value is passed by reference, so that the iterator can restore it when breaking out of the loop. Additionally it moves the AOP_WRITEPAGE_ACTIVATE out of the iterator and into the callers, in preparation for eventually killing it off with the phase out of write_cache_pages(). To me this form of the loop feels easier to follow, and it has the added advantage that writeback_iter() can actually be nicely used in nested loops, which should help with further iterizing the iomap writeback code. Signed-off-by: Christoph Hellwig Reviewed-by: Jan Kara --- fs/iomap/buffered-io.c | 7 +- include/linux/writeback.h | 11 +-- mm/page-writeback.c | 174 +++++++++++++++++++++----------------- 3 files changed, 102 insertions(+), 90 deletions(-) diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 58b3661f5eac9e..1593a783176ca2 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -1985,12 +1985,13 @@ iomap_writepages(struct address_space *mapping, struct writeback_control *wbc, struct iomap_writepage_ctx *wpc, const struct iomap_writeback_ops *ops) { - struct folio *folio; - int ret; + struct folio *folio = NULL; + int ret = 0; wpc->ops = ops; - for_each_writeback_folio(mapping, wbc, folio, ret) + while ((folio = writeback_iter(mapping, wbc, folio, &ret))) ret = iomap_do_writepage(folio, wbc, wpc); + if (!wpc->ioend) return ret; return iomap_submit_ioend(wpc, wpc->ioend, ret); diff --git a/include/linux/writeback.h b/include/linux/writeback.h index 2416da933440e2..fc4605627496fc 100644 --- a/include/linux/writeback.h +++ b/include/linux/writeback.h @@ -367,15 +367,8 @@ int balance_dirty_pages_ratelimited_flags(struct address_space *mapping, bool wb_over_bg_thresh(struct bdi_writeback *wb); -struct folio *writeback_iter_init(struct address_space *mapping, - struct writeback_control *wbc); -struct folio *writeback_iter_next(struct address_space *mapping, - struct writeback_control *wbc, struct folio *folio, int error); - -#define for_each_writeback_folio(mapping, wbc, folio, error) \ - for (folio = writeback_iter_init(mapping, wbc); \ - folio || ((error = wbc->err), false); \ - folio = writeback_iter_next(mapping, wbc, folio, error)) +struct folio *writeback_iter(struct address_space *mapping, + struct writeback_control *wbc, struct folio *folio, int *error); typedef int (*writepage_t)(struct folio *folio, struct writeback_control *wbc, void *data); diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 2a4b5aee5decd9..9e1cce9be63524 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2360,29 +2360,6 @@ void tag_pages_for_writeback(struct address_space *mapping, } EXPORT_SYMBOL(tag_pages_for_writeback); -static void writeback_finish(struct address_space *mapping, - struct writeback_control *wbc, pgoff_t done_index) -{ - folio_batch_release(&wbc->fbatch); - - /* - * For range cyclic writeback we need to remember where we stopped so - * that we can continue there next time we are called. If we hit the - * last page and there is more work to be done, wrap back to the start - * of the file. - * - * For non-cyclic writeback we always start looking up at the beginning - * of the file if we are called again, which can only happen due to - * -ENOMEM from the file system. - */ - if (wbc->range_cyclic) { - if (wbc->err || wbc->nr_to_write <= 0) - mapping->writeback_index = done_index; - else - mapping->writeback_index = 0; - } -} - static xa_mark_t wbc_to_tag(struct writeback_control *wbc) { if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages) @@ -2442,10 +2419,8 @@ static struct folio *writeback_get_folio(struct address_space *mapping, filemap_get_folios_tag(mapping, &wbc->index, wbc_end(wbc), wbc_to_tag(wbc), &wbc->fbatch); folio = folio_batch_next(&wbc->fbatch); - if (!folio) { - writeback_finish(mapping, wbc, 0); + if (!folio) return NULL; - } } folio_lock(folio); @@ -2458,60 +2433,92 @@ static struct folio *writeback_get_folio(struct address_space *mapping, return folio; } -struct folio *writeback_iter_init(struct address_space *mapping, - struct writeback_control *wbc) -{ - if (wbc->range_cyclic) - wbc->index = mapping->writeback_index; /* prev offset */ - else - wbc->index = wbc->range_start >> PAGE_SHIFT; - - if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages) - tag_pages_for_writeback(mapping, wbc->index, wbc_end(wbc)); - - wbc->err = 0; - folio_batch_init(&wbc->fbatch); - return writeback_get_folio(mapping, wbc); -} - -struct folio *writeback_iter_next(struct address_space *mapping, - struct writeback_control *wbc, struct folio *folio, int error) +/** + * writepage_iter - iterate folio of a mapping for writeback + * @mapping: address space structure to write + * @wbc: writeback context + * @folio: previously iterated folio (%NULL to start) + * @error: in-out pointer for writeback errors (see below) + * + * This function should be called in a while loop in the ->writepages + * implementation and returns the next folio for the writeback operation + * described by @wbc on @mapping. + * + * To start writeback @folio should be passed as NULL, for every following + * iteration the folio returned by this function previously should be passed. + * @error should contain the error from the previous writeback iteration when + * calling writeback_iter. + * + * Once the writeback described in @wbc has finished, this function will return + * %NULL and if there was an error in any iteration restore it to @error. + * + * Note: callers should not manually break out of the loop using break or goto. + */ +struct folio *writeback_iter(struct address_space *mapping, + struct writeback_control *wbc, struct folio *folio, int *error) { - unsigned long nr = folio_nr_pages(folio); + if (folio) { + wbc->nr_to_write -= folio_nr_pages(folio); + if (*error && !wbc->err) + wbc->err = *error; - wbc->nr_to_write -= nr; - - /* - * Handle the legacy AOP_WRITEPAGE_ACTIVATE magic return value. - * Eventually all instances should just unlock the folio themselves and - * return 0; - */ - if (error == AOP_WRITEPAGE_ACTIVATE) { - folio_unlock(folio); - error = 0; + /* + * For integrity sync we have to keep going until we have + * written all the folios we tagged for writeback prior to + * entering the writeback loop, even if we run past + * wbc->nr_to_write or encounter errors. + * + * This is because the file system may still have state to clear + * for each folio. We'll eventually return the first error + * encountered. + * + * For background writeback just push done_index past this folio + * so that we can just restart where we left off and media + * errors won't choke writeout for the entire file. + */ + if (wbc->sync_mode == WB_SYNC_NONE && + (wbc->err || wbc->nr_to_write <= 0)) + goto finish; + } else { + if (wbc->range_cyclic) + wbc->index = mapping->writeback_index; /* prev offset */ + else + wbc->index = wbc->range_start >> PAGE_SHIFT; + if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages) + tag_pages_for_writeback(mapping, wbc->index, + wbc_end(wbc)); + folio_batch_init(&wbc->fbatch); + wbc->err = 0; } - if (error && !wbc->err) - wbc->err = error; + folio = writeback_get_folio(mapping, wbc); + if (!folio) + goto finish; + return folio; + +finish: + folio_batch_release(&wbc->fbatch); /* - * For integrity sync we have to keep going until we have written all - * the folios we tagged for writeback prior to entering the writeback - * loop, even if we run past wbc->nr_to_write or encounter errors. - * This is because the file system may still have state to clear for - * each folio. We'll eventually return the first error encountered. + * For range cyclic writeback we need to remember where we stopped so + * that we can continue there next time we are called. If we hit the + * last page and there is more work to be done, wrap back to the start + * of the file. * - * For background writeback just push done_index past this folio so that - * we can just restart where we left off and media errors won't choke - * writeout for the entire file. + * For non-cyclic writeback we always start looking up at the beginning + * of the file if we are called again, which can only happen due to + * -ENOMEM from the file system. */ - if (wbc->sync_mode == WB_SYNC_NONE && - (wbc->err || wbc->nr_to_write <= 0)) { - writeback_finish(mapping, wbc, folio->index + nr); - return NULL; + if (wbc->range_cyclic) { + WARN_ON_ONCE(wbc->sync_mode != WB_SYNC_NONE); + if (wbc->err || wbc->nr_to_write <= 0) + mapping->writeback_index = + folio->index + folio_nr_pages(folio); + else + mapping->writeback_index = 0; } - - return writeback_get_folio(mapping, wbc); + *error = wbc->err; + return NULL; } /** @@ -2549,13 +2556,18 @@ int write_cache_pages(struct address_space *mapping, struct writeback_control *wbc, writepage_t writepage, void *data) { - struct folio *folio; - int error; + struct folio *folio = NULL; + int error = 0; - for_each_writeback_folio(mapping, wbc, folio, error) + while ((folio = writeback_iter(mapping, wbc, folio, &error))) { error = writepage(folio, wbc, data); + if (error == AOP_WRITEPAGE_ACTIVATE) { + folio_unlock(folio); + error = 0; + } + } - return wbc->err; + return error; } EXPORT_SYMBOL(write_cache_pages); @@ -2563,13 +2575,17 @@ static int writeback_use_writepage(struct address_space *mapping, struct writeback_control *wbc) { struct blk_plug plug; - struct folio *folio; - int err; + struct folio *folio = 0; + int err = 0; blk_start_plug(&plug); - for_each_writeback_folio(mapping, wbc, folio, err) { + while ((folio = writeback_iter(mapping, wbc, folio, &err))) { err = mapping->a_ops->writepage(&folio->page, wbc); mapping_set_error(mapping, err); + if (err == AOP_WRITEPAGE_ACTIVATE) { + folio_unlock(folio); + err = 0; + } } blk_finish_plug(&plug); @@ -2590,6 +2606,8 @@ int do_writepages(struct address_space *mapping, struct writeback_control *wbc) ret = mapping->a_ops->writepages(mapping, wbc); } else if (mapping->a_ops->writepage) { ret = writeback_use_writepage(mapping, wbc); + if (!ret) + ret = wbc->err; } else { /* deal with chardevs and other special files */ ret = 0;