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 61E19C41535 for ; Tue, 19 Dec 2023 18:27:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C94B16B0089; Tue, 19 Dec 2023 13:27:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C41886B008A; Tue, 19 Dec 2023 13:27:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id ABB596B008C; Tue, 19 Dec 2023 13:27:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 92D586B0089 for ; Tue, 19 Dec 2023 13:27:16 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 6E256160236 for ; Tue, 19 Dec 2023 18:27:16 +0000 (UTC) X-FDA: 81584400072.24.7B92300 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf07.hostedemail.com (Postfix) with ESMTP id 1D2BD40005 for ; Tue, 19 Dec 2023 18:27:13 +0000 (UTC) Authentication-Results: imf07.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=yJkIpqLj; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=ND3r4peh; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=yJkIpqLj; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=ND3r4peh; dmarc=none; spf=pass (imf07.hostedemail.com: domain of jack@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1703010434; 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=8wwwn4EPAgSsB+knSVTXoG3tIQt4g4UOJHmQAEbH/lw=; b=EBE1rrbC8ELwB2ebJGHlm5xqrpsqPIBzgtW4QS7vaaL7CdYLeTE9g4aE5aXcqQeJI20WAA n3tzM/SeCbAu5mUIq3NCaU+ZK9kJ0J12NGvbvWwN9DRc9kWzClNl7G8Qrlc9CQ/luD8ome 9aLrdIH0FIdm4+m5V3B0dTc246nHI0E= ARC-Authentication-Results: i=1; imf07.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=yJkIpqLj; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=ND3r4peh; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=yJkIpqLj; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=ND3r4peh; dmarc=none; spf=pass (imf07.hostedemail.com: domain of jack@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1703010434; a=rsa-sha256; cv=none; b=C0aG6z2pOOkvuFEPkubnhp8HF9jarKgVwRyB4juH/TCJ8stWc36JZzfvjLRnMB45cD7tDb 5C73wpItAWiR6jRgVwXZYH/7Gnk9T0MK1dBXlxBJ9U+sF5D88tj3Rt0d0bILFPOOVf6/Bw ARb0HjrcFCkWGpdU4XPu+b9Ly0CO5V4= Received: from imap2.dmz-prg2.suse.org (imap2.dmz-prg2.suse.org [10.150.64.98]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 4485E22168; Tue, 19 Dec 2023 18:27:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1703010432; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8wwwn4EPAgSsB+knSVTXoG3tIQt4g4UOJHmQAEbH/lw=; b=yJkIpqLjMNi3Dj3rOmN1hIIVyNJGeRo9IEl/0PXk6uIc8ickQFNyOdAXckJE/lFFWzei68 jiA+UhucvHxP77vqPKCX8yefHPJHw8ectoRREJB55q4/rsRoJ8ke/SsTvNCmV35SBskjk5 rXzGAyu9OpT1jPMTJtGplFMM4NaUK4g= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1703010432; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8wwwn4EPAgSsB+knSVTXoG3tIQt4g4UOJHmQAEbH/lw=; b=ND3r4peh60mJqstnuduW+WeCtT/aPlEPjUdS5JlHdaX3N5OAzE8MHokT63Kk2/8lyRIO1q 5Ct13LGe8WOshGCw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1703010432; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8wwwn4EPAgSsB+knSVTXoG3tIQt4g4UOJHmQAEbH/lw=; b=yJkIpqLjMNi3Dj3rOmN1hIIVyNJGeRo9IEl/0PXk6uIc8ickQFNyOdAXckJE/lFFWzei68 jiA+UhucvHxP77vqPKCX8yefHPJHw8ectoRREJB55q4/rsRoJ8ke/SsTvNCmV35SBskjk5 rXzGAyu9OpT1jPMTJtGplFMM4NaUK4g= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1703010432; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=8wwwn4EPAgSsB+knSVTXoG3tIQt4g4UOJHmQAEbH/lw=; b=ND3r4peh60mJqstnuduW+WeCtT/aPlEPjUdS5JlHdaX3N5OAzE8MHokT63Kk2/8lyRIO1q 5Ct13LGe8WOshGCw== Received: from imap2.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap2.dmz-prg2.suse.org (Postfix) with ESMTPS id 3207A13BF1; Tue, 19 Dec 2023 18:27:12 +0000 (UTC) Received: from dovecot-director2.suse.de ([10.150.64.162]) by imap2.dmz-prg2.suse.org with ESMTPSA id byzsC4DggWVLLgAAn2gu4w (envelope-from ); Tue, 19 Dec 2023 18:27:12 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id 790E5A07E0; Tue, 19 Dec 2023 19:27:11 +0100 (CET) Date: Tue, 19 Dec 2023 19:27:11 +0100 From: Jan Kara To: Christoph Hellwig Cc: linux-mm@kvack.org, "Matthew Wilcox (Oracle)" , Jan Kara , David Howells , Brian Foster , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 03/17] writeback: rework the loop termination condition in write_cache_pages Message-ID: <20231219182711.oskwl65vdctbpsxe@quack3> References: <20231218153553.807799-1-hch@lst.de> <20231218153553.807799-4-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231218153553.807799-4-hch@lst.de> X-Rspamd-Queue-Id: 1D2BD40005 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: g8uaxca8dpf47y75nzgyh5384kuhwayu X-HE-Tag: 1703010433-822486 X-HE-Meta: U2FsdGVkX1+M96Yrz8KKYAmndtfCZzbColVCfUipbkoKJr7DVGqUgj2Pc7CwDJmAyQr4SoA0YZsuEMhC6fBsD/KtMSrgL2t2M96u6dtlvMG6Tw9/TLSqW4Ab+qKI/M60kNpckLLbklVWjxuM647y6dz4wCSknWugpr/TdDJzfa0Ymv4TtPDh+s4IytOVKNw6v3yN7Kryu9ZeOPYIGvA3iDO2zM6bdmQzEeP1pKQtkHMvM2wQQOErj9fCWGBRWJB1LNLFpIwIiubEH1RzGHfd28YEOQ5XvWlvZtWxnKuCFIiY+QQMnDqi4tZvzZJaytBOloSMLbRu/TMdJ/8tZ2AQSuFpoD5a8JnEh2TxF4qgbPkU9k2O6+OkJSXivCf54YEz1X1ef8+rL0+m4aOdj/f7b91cza0w/u66Ht33rO0Pn6tTvJAEGGHZAtqdUZiKB7MSbCyoHyJfjEbTDF7PmzdssQFwSav7Nx3MCS3rQSiB8waRogDpJIyosi3Qq5zDO/4QNocT5uiRUykoQQPKjAbY36wchA0A2c5D24E1/NWAwKtUlVWY86YrXaud+KgHU/oPAa35wssdfU6m13MwyiPRjXw9lWexAKdkfS3GFewv+ipI9D3jOhjRz9R2Q3qJxnlVgrmKXfsfPqfF0zihLychdbh8jw3sAlO5H1zXIYZSpmnW23OuWMKbFDdr4Guz7dPLkz0TxggY8LyAROfOnsuQBcMdM9IRJ3b33G2qGLeHmo6/zRuA4T4sRvOWFIJV20KWcD4WrcPudFlICJjz52N6DMT0Xh27i/E3ujFJnCOd10ebdbh/A9RE3JqWU5KbytVdDgXKsMbM14Ri6ghZQDU0kxbqtL/56QYlDEfVc2h79I0Uh99JOdpVpy3uOqSmpiUndHFxUN/jmIDTgeRRLGwXPitxHsbmnkbmVUwLKigGFVFsK14E0/tG/vbJM9kBgT/o 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: On Mon 18-12-23 16:35:39, Christoph Hellwig wrote: > Rework we deal with the cleanup after the writepage call. First handle ^^ the way > the magic AOP_WRITEPAGE_ACTIVATE separately from real error returns to > get it out of the way of the actual error handling. Then merge the > code to set ret for integrity vs non-integrity writeback. For > non-integrity writeback the loop is terminated on the first error, so > ret will never be non-zero. Then use a single block to check for > non-integrity writewack to consolidate the cases where it returns for > either an error or running off the end of nr_to_write. > > Signed-off-by: Christoph Hellwig Otherwise looks good. Feel free to add: Reviewed-by: Jan Kara Honza > --- > mm/page-writeback.c | 62 +++++++++++++++++++++------------------------ > 1 file changed, 29 insertions(+), 33 deletions(-) > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 8e312d73475646..7ed6c2bc8dd51c 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2474,43 +2474,39 @@ int write_cache_pages(struct address_space *mapping, > error = writepage(folio, wbc, data); > nr = folio_nr_pages(folio); > wbc->nr_to_write -= nr; > - if (unlikely(error)) { > - /* > - * Handle errors according to the type of > - * writeback. There's no need to continue for > - * background writeback. Just push done_index > - * past this page so media errors won't choke > - * writeout for the entire file. For integrity > - * writeback, we must process the entire dirty > - * set regardless of errors because the fs may > - * still have state to clear for each page. In > - * that case we continue processing and return > - * the first error. > - */ > - if (error == AOP_WRITEPAGE_ACTIVATE) { > - folio_unlock(folio); > - error = 0; > - } else if (wbc->sync_mode != WB_SYNC_ALL) { > - ret = error; > - done_index = folio->index + nr; > - done = 1; > - break; > - } > - if (!ret) > - ret = error; > + > + /* > + * 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; > } > > /* > - * We stop writing back only if we are not doing > - * integrity sync. In case of integrity sync we have to > - * keep going until we have written all the pages > - * we tagged for writeback prior to entering this loop. > + * For integrity sync we have to keep going until we > + * have written all the folios we tagged for writeback > + * prior to entering this 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. > */ > - done_index = folio->index + nr; > - if (wbc->nr_to_write <= 0 && > - wbc->sync_mode == WB_SYNC_NONE) { > - done = 1; > - break; > + if (error && !ret) > + ret = error; > + if (wbc->sync_mode == WB_SYNC_NONE) { > + if (ret || wbc->nr_to_write <= 0) { > + done_index = folio->index + nr; > + done = 1; > + break; > + } > } > } > folio_batch_release(&fbatch); > -- > 2.39.2 > -- Jan Kara SUSE Labs, CR