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 0659BC35274 for ; Fri, 15 Dec 2023 14:25:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9C7B28D012F; Fri, 15 Dec 2023 09:25:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 977D48D0121; Fri, 15 Dec 2023 09:25:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 83F4D8D012F; Fri, 15 Dec 2023 09:25:53 -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 757DC8D0121 for ; Fri, 15 Dec 2023 09:25:53 -0500 (EST) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 4F814A22C5 for ; Fri, 15 Dec 2023 14:25:53 +0000 (UTC) X-FDA: 81569276586.26.130CB17 Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by imf14.hostedemail.com (Postfix) with ESMTP id A73B9100017 for ; Fri, 15 Dec 2023 14:25:50 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=fV3YPsKf; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=cQxuTYfK; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=M5nZCg9o; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=PzffO8tk; spf=pass (imf14.hostedemail.com: domain of jack@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=jack@suse.cz; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702650351; 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=O+1Vzn6gSgVRj2PeTCSSeHhZYkHtT8mht0lDEDG5nPo=; b=WhZprupjg4gTUB4/Xo9pk9bNqoVr28jAwWlYUzjpPPlfJs2tX/igfuVAfUs0eexxXJiQ6O 4Tq+2a0H7z4kwnAC2+QzYr5bYvaTmetUKOwuXgeoEoRvRisDBDwfyn+xdE66mQrcMbZ1SS s/UFUinPRKl/ZZt3kyFhamK/i50g9uk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702650351; a=rsa-sha256; cv=none; b=inUHD9bG87zrG9RpMN7A8vFInubO59TxGVdijHDMlAN4GeitAyI2PQEdxWi115tC+CwIVA r5cF+YbFn9O0Mn/hzRtMMvdICEDTFoFrL3At9PC/bMlJtFkoq0/aGZYHzwebNAWLVw4YUX ium4hmTSdwX1wdHaNMvU4LbRxaFSyfI= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=fV3YPsKf; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=cQxuTYfK; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=M5nZCg9o; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=PzffO8tk; spf=pass (imf14.hostedemail.com: domain of jack@suse.cz designates 195.135.223.130 as permitted sender) smtp.mailfrom=jack@suse.cz; dmarc=none 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 B25E821B39; Fri, 15 Dec 2023 14:25:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1702650348; 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=O+1Vzn6gSgVRj2PeTCSSeHhZYkHtT8mht0lDEDG5nPo=; b=fV3YPsKfDjKk0wDiNz5lHnGWIiJvWJn3v5VRSZuiXcWcbcQsSTebimcXCwNzY6DwLoI4el NmkJEaGodEOhM2aK41i5PzERVhj62TqTcF7daDMICfL9lPmZEQs9F54TnsLcrObmCnIUiy DpS629c45/MhykftG8M/OZV6hrY1dMY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1702650348; 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=O+1Vzn6gSgVRj2PeTCSSeHhZYkHtT8mht0lDEDG5nPo=; b=cQxuTYfKDmFDq0YwbAMo96JMXs7XZYpTd0yp/UY6DnsDk2q75hWZgZw44jyVmMkdzDl1y1 LuAETYpHzNqfG4BA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1702650347; 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=O+1Vzn6gSgVRj2PeTCSSeHhZYkHtT8mht0lDEDG5nPo=; b=M5nZCg9oo12FS/a8eOsTeKRws1NkrZnAooEh/8YzOCZ4gEyXU3Nsd766n3YXrZzSTZYlFt sx/2zsjMbQcVFtRpzS8eJiYvE/zi1Q5AEQ0A9d/LuUuZfuF8LGWv/Pj46YNYEbo2T1UXXf IiIasiVRYe4DE4goUvHCEc4sXbXl90E= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1702650347; 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=O+1Vzn6gSgVRj2PeTCSSeHhZYkHtT8mht0lDEDG5nPo=; b=PzffO8tkAWptGRWe97c2p1cCLFw+S+acfO2t/aHorPPxZFPrWHt3ywkJvbSt11/5yEVg7k S55j3eGI9/7we0CQ== 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 A18E113A08; Fri, 15 Dec 2023 14:25:47 +0000 (UTC) Received: from dovecot-director2.suse.de ([10.150.64.162]) by imap2.dmz-prg2.suse.org with ESMTPSA id HC0rJ+thfGWeawAAn2gu4w (envelope-from ); Fri, 15 Dec 2023 14:25:47 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id 286B7A07E0; Fri, 15 Dec 2023 15:25:47 +0100 (CET) Date: Fri, 15 Dec 2023 15:25:47 +0100 From: Jan Kara To: Christoph Hellwig Cc: linux-mm@kvack.org, "Matthew Wilcox (Oracle)" , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Jan Kara , David Howells Subject: Re: [PATCH 04/11] writeback: Simplify the loops in write_cache_pages() Message-ID: <20231215142547.46g2lqs2d2u3ljwl@quack3> References: <20231214132544.376574-1-hch@lst.de> <20231214132544.376574-5-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231214132544.376574-5-hch@lst.de> X-Rspamd-Queue-Id: A73B9100017 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: 6iqzib5k8u5nwy4r4xurfsrr4pbc1e85 X-HE-Tag: 1702650350-794372 X-HE-Meta: U2FsdGVkX19CyV5Dl/7st5jPP4pdH6wt+5OUVzYxncrQ7lSpIejpRO4KWUUqtQBg3hrtI4TicfB41akWOrw8BkNfFiaRc7LTRNAufsxtJYK7JrsKJGF7otf0zzaaxp3Miw1NtYaEXKHzcFJOCrqzwkmXi776IpR2cq2eRo5vzsxwwMQoLp3zq2tuPIwoQ5h8GealdSiqtAI0WcH7fCEV7/62g7f85rFyD0e9HSjbGc5ugLz1nTzpu997bK+6HNRoN2nQHhXgccV3l6M11JRqDsmah0MMDz4I/KGaZjXw70FsAMTJbDBmAoPOqJXKZu5luQXu/af/Wy5h8mwwLMktvRaomjrc83GUH6fsubTXWQDVb2+uIPttfSizQIQ3m6T8JuaAKsBMk7hYcUTLEvZffo7x94OdZI5i4mBEosuXcJKmUup7SeKVe5jbM9YSC6H4vpO5OL9VEftiqWq55iQ/Tyg1+IvrNDUd9SlJNrUKIFIyykr94DqpHe5PGhBDt+KXeBq/i3/Lsp6zPu7rtozvN8QyUi6HdHs8dL71/x5cuW67Sp51xmQNFgc+wB3Kr4omGWSCd7PovuHb7ASHa035PnXceAS7f67nkRx10lp4kTgRNCpPKuGFz9HX4Z8MBA1OBSXUISKolfXma9eHASujQcxOmyjivBTC/MP3vwPg5kHtf1lwmnnfn+FNVvU4l6+I+7fhSOtHiIYoH1oDw4n4USlhTMUbEfPmTP3Oj/A+kKQQT6Sh0+b3165pFJ103E+PpnrSGha1suE6zxgDRWiGiP78mmkki4KJdvyKQ+pdJ4FIB9vIV+X0JVAr+qRRRPC42yUHlfcXZZKAAV/V69KTjlZzacLnYKvDlUph85RnAKgPekucfJAYCzE9CjmVOLmITtpFZK1tAyl0umFMmH5nimHR3P4SzHVRYILZa2pFgazCe4m1SUU9HyQGNiftQVmQ8r7nvu/ayqsgdxcZ9i8 C3RZQgwb Hg82pgDMTjAZwWxlJQNKrDyAinkMVFEApirRYkgPhZBBffvNNGJZwRHlTETs9lXkLI8nm9Hii+wYmGgmzJ1xwDr+9XweS9ydAii1/sIctsOQnlZ/z7rEeF4vMvTA32PbDmFOZFxYe3IGeVS4QGQhqP7R//1yV1a6DaDywJof+6puqEVzmKq0ZnVBhmkfRcvKtkNCjrhzsm5FC7t2YX7qhyDC5D6IjlRpKawG6/UX7ZfHlhK1m+903Ke5wpcsuNOIh0Xt0WI/7o24WsCmkc5xu/dQvvQOOuBj5IEduLJ0uvjdPB7mqskgrpgRh+fNKuh/2WoDOXGKpChUHlNszEhd2TSXGUHZZm/UIrUGx+s87yhfMsow= 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 Thu 14-12-23 14:25:37, Christoph Hellwig wrote: > From: "Matthew Wilcox (Oracle)" > > Collapse the two nested loops into one. This is needed as a step > towards turning this into an iterator. > > Signed-off-by: Christoph Hellwig It would be good to mention in the changelog that we drop the condition index <= end and just rely on filemap_get_folios_tag() to return 0 entries when index > end. This actually has a subtle implication when end == -1 because then the returned index will be -1 as well and thus if there is page present on index -1, we could be looping indefinitely. But I think that's mostly a theoretical concern so I'd be fine with just mentioning this subtlety in the changelog and possibly in a comment in the code. Honza > --- > mm/page-writeback.c | 98 ++++++++++++++++++++++----------------------- > 1 file changed, 49 insertions(+), 49 deletions(-) > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index 5a3df8665ff4f9..2087d16115710e 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2460,6 +2460,7 @@ int write_cache_pages(struct address_space *mapping, > void *data) > { > int error; > + int i = 0; > > if (wbc->range_cyclic) { > wbc->index = mapping->writeback_index; /* prev offset */ > @@ -2477,67 +2478,66 @@ int write_cache_pages(struct address_space *mapping, > folio_batch_init(&wbc->fbatch); > wbc->err = 0; > > - while (wbc->index <= wbc->end) { > - int i; > - > - writeback_get_batch(mapping, wbc); > + for (;;) { > + struct folio *folio; > + unsigned long nr; > > + if (i == wbc->fbatch.nr) { > + writeback_get_batch(mapping, wbc); > + i = 0; > + } > if (wbc->fbatch.nr == 0) > break; > > - for (i = 0; i < wbc->fbatch.nr; i++) { > - struct folio *folio = wbc->fbatch.folios[i]; > - unsigned long nr; > + folio = wbc->fbatch.folios[i++]; > > - wbc->done_index = folio->index; > + wbc->done_index = folio->index; > > - folio_lock(folio); > - if (!should_writeback_folio(mapping, wbc, folio)) { > - folio_unlock(folio); > - continue; > - } > + folio_lock(folio); > + if (!should_writeback_folio(mapping, wbc, folio)) { > + folio_unlock(folio); > + continue; > + } > > - trace_wbc_writepage(wbc, inode_to_bdi(mapping->host)); > - > - error = writepage(folio, wbc, data); > - nr = folio_nr_pages(folio); > - 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) { > - wbc->err = error; > - wbc->done_index = folio->index + nr; > - return writeback_finish(mapping, > - wbc, true); > - } > - if (!wbc->err) > - wbc->err = error; > - } > + trace_wbc_writepage(wbc, inode_to_bdi(mapping->host)); > > + error = writepage(folio, wbc, data); > + nr = folio_nr_pages(folio); > + if (unlikely(error)) { > /* > - * 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. > + * 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. > */ > - wbc->nr_to_write -= nr; > - if (wbc->nr_to_write <= 0 && > - wbc->sync_mode == WB_SYNC_NONE) > + if (error == AOP_WRITEPAGE_ACTIVATE) { > + folio_unlock(folio); > + error = 0; > + } else if (wbc->sync_mode != WB_SYNC_ALL) { > + wbc->err = error; > + wbc->done_index = folio->index + nr; > return writeback_finish(mapping, wbc, true); > + } > + if (!wbc->err) > + wbc->err = error; > } > + > + /* > + * 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. > + */ > + wbc->nr_to_write -= nr; > + if (wbc->nr_to_write <= 0 && > + wbc->sync_mode == WB_SYNC_NONE) > + return writeback_finish(mapping, wbc, true); > } > > return writeback_finish(mapping, wbc, false); > -- > 2.39.2 > -- Jan Kara SUSE Labs, CR