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 C2642C35274 for ; Fri, 15 Dec 2023 14:16:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5C9CA6B0629; Fri, 15 Dec 2023 09:16:13 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 579FA6B062A; Fri, 15 Dec 2023 09:16:13 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4690B6B062B; Fri, 15 Dec 2023 09:16:13 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 373916B0629 for ; Fri, 15 Dec 2023 09:16:13 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 014938058F for ; Fri, 15 Dec 2023 14:16:12 +0000 (UTC) X-FDA: 81569252226.14.BE7A3B1 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf29.hostedemail.com (Postfix) with ESMTP id 0F09B120012 for ; Fri, 15 Dec 2023 14:16:10 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="W5a/K6zM"; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf29.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702649771; 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=LGCo/BpL11RGikah8KaGUvZ8gRuDuiWWsX0i2uxlHCs=; b=WSfiq0OjWC2AW09c4M3FQWeZ9Eel5PMhdoSmt2oZGjfA0268nwROQp8OLsFSjImhLuEOtV LRfKXv842CQE8NqCvHJCOskcO/jECuEmoUu9YKFdXC9WuDZLlWRvAbLmUn+9J6URi6o9Q3 0UpBi3dmBMVhn1QQFzpiV2iGgEIhrqc= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b="W5a/K6zM"; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf29.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702649771; a=rsa-sha256; cv=none; b=w8rhaVPNTQXbRoSy7Ru3BPQo68nCLp023q3cPMxIeVbLEacSd4ma4fSKJBrlntb23qr8lh 0fsI++QOOTHqZ67TtKYj2n+iC9KgjAFO/8qYl6yEtFyNENpEiWSIrxcg/Jb8MzjGXT+nNN mIQ0ybrZYLHc4Z3qS3fufjHksHjdKjo= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1702649770; h=from:from:reply-to:subject:subject: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=LGCo/BpL11RGikah8KaGUvZ8gRuDuiWWsX0i2uxlHCs=; b=W5a/K6zMFijxzDm4vTZwre49XwiSDo599YeinQVmddcy5oaEX+k5gqR+TgLz58qbGWZRYd QZnf0BFPgOc6I/ZkdOveD6l/3YxPMHHNQ/GffHvWSDe2nh7KmCegPs7ABLUSHSBoD4NvjO yjwQAzZ/YZ/I9T27kdPix4ANA8LQSiw= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-655-f1soc7hsN0ywK75HVsslug-1; Fri, 15 Dec 2023 09:16:05 -0500 X-MC-Unique: f1soc7hsN0ywK75HVsslug-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id BD4DE185A784; Fri, 15 Dec 2023 14:16:04 +0000 (UTC) Received: from bfoster (unknown [10.22.8.199]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 313A81C060B1; Fri, 15 Dec 2023 14:16:04 +0000 (UTC) Date: Fri, 15 Dec 2023 09:17:05 -0500 From: Brian Foster 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 09/11] writeback: Factor writeback_iter_next() out of write_cache_pages() Message-ID: References: <20231214132544.376574-1-hch@lst.de> <20231214132544.376574-10-hch@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231214132544.376574-10-hch@lst.de> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Rspamd-Queue-Id: 0F09B120012 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: dx13w9gm53fgkebsozzfy97g37enjupa X-HE-Tag: 1702649770-470356 X-HE-Meta: U2FsdGVkX1++iwsUmW6Rx7HLx/7e4cB4ypDVtGf4z5/HrdwDAluSRgeyvTgB1sdeJ1KpQfosJuo44Uud3TmIsODl1kmiQltbQEctg9EYV+Wf/JVvu+M9AWMQD2qx2z1KLuUjMkEl6ZEEHsPD8mzyKrz/bZRFDgnd/EpkKU0hMQ4GiwsWfBoMran6p1w85slYJnTpB6bj6EUqPUzb7MLakuTUAB0/I3OOhDge+Bjq4dTIMoeVurepPajdSL6mXCA5AIs15lCDL/kj5EZVgw6kESzmgaVvJ0I3nDMNniz4Yp2rMacPqbIJDJrE6fU2cj/28t/s+G24m+GYbD/g8ZSqx15/AIyRhYEZttmKQeW+mW7Aw1DRZZVKC9WLVbC8MPtibZldiVio3oeHb+wf3parv33vOboCuBvjZRLaX14Vh8PumlnC2EaVesjwS30PQi6B0J2dn3x4JSwe5w8whiyKLS2yPB0th6UKlY1d3Sx4enj8e6iR2lFneNnPDrTSaMm7zcv+Nl97XWTV6lWW00dVQ4DDkfIRKUW4F5mr6tEiGrMwaGdOrTkgwZiWHuBXQKGQRwGGHgsnElhZYr61Y60+MXQqsGWMiPpq736lDpez6lo5R0MuqkJnpyag1hywQyX5j7uxftnageYpV9SE7hVq1MNpqbMFhgm32KLaNAJTnu8WlWQ2D1P5z//Ccu80arboZjvB0sD8kJ65c2osE3gST1YqDcmkc44NuJmFwL8lved65cT3qfFWenVnm+ndnQzVlWjohDyVJVC6rf3kl3oeF9QpFDvrODGsMsmYPTnhzo+jgbHlsTMpNi2bMAuo3D/8L93Sh1yTghNYUK1gHcai0oOaEIT0rfOqUYNnY562xUo2z0IOH0ATJY6sozzrQjs54xsGiTJi1hnphus1QSTZt63fHXebux7V+eGhRyggPEH8S+8EWamGfqBLK384nK3ee/MgxhD+GJccb23dXgq XdW8VOBU i6c6SzeDhfD5DDunUTEUdjJhcQWxO0/JaxNWBb0LOh43bjexi2i3M4ehZbRKnVb6iHHlMMAgqAhB4wJwyuyU2ExFqeIK+0LAAbIoASfi6M3mlbTS9uVmnmvlHuIW9jUkmlejAT2BWkAx4GTrrXAdUF8g1pZVb7Pd+CBZ7lNlA4Cl7IyAVQ7XNX+b9XLokKVMAnm34IQHY3WkZAcTfwgV5kvg2lfkUow1nBGSks9u7j0fU9CLgFCJZT0E0V7j03/s7HpEXHrOEztskFj0= 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, Dec 14, 2023 at 02:25:42PM +0100, Christoph Hellwig wrote: > From: "Matthew Wilcox (Oracle)" > > Pull the post-processing of the writepage_t callback into a > separate function. That means changing writeback_finish() to > return NULL, and writeback_get_next() to call writeback_finish() > when we naturally run out of folios. > > Signed-off-by: Matthew Wilcox (Oracle) > Signed-off-by: Christoph Hellwig > --- > mm/page-writeback.c | 89 +++++++++++++++++++++++---------------------- > 1 file changed, 46 insertions(+), 43 deletions(-) > > diff --git a/mm/page-writeback.c b/mm/page-writeback.c > index b0accca1f4bfa7..4fae912f7a86e2 100644 > --- a/mm/page-writeback.c > +++ b/mm/page-writeback.c > @@ -2360,7 +2360,7 @@ void tag_pages_for_writeback(struct address_space *mapping, > } > EXPORT_SYMBOL(tag_pages_for_writeback); > > -static int writeback_finish(struct address_space *mapping, > +static struct folio *writeback_finish(struct address_space *mapping, > struct writeback_control *wbc, bool done) > { > folio_batch_release(&wbc->fbatch); > @@ -2375,7 +2375,7 @@ static int writeback_finish(struct address_space *mapping, > if (wbc->range_cyclic || (wbc->range_whole && wbc->nr_to_write > 0)) > mapping->writeback_index = wbc->done_index; > > - return wbc->err; > + return NULL; The series looks reasonable to me on a first pass, but this stood out to me as really odd. I was initially wondering if it made sense to use an ERR_PTR() here or something, but on further staring it kind of seems like this is better off being factored out of the internal iteration paths. Untested and Probably Broken patch (based on this one) below as a quick reference, but just a thought... BTW it would be nicer to just drop the ->done field entirely as Jan has already suggested, but I just stuffed it in wbc for simplicity. Brian --- 8< --- diff --git a/include/linux/writeback.h b/include/linux/writeback.h index be960f92ad9d..0babca17a2c0 100644 --- a/include/linux/writeback.h +++ b/include/linux/writeback.h @@ -84,6 +84,7 @@ struct writeback_control { pgoff_t index; pgoff_t end; /* inclusive */ pgoff_t done_index; + bool done; int err; unsigned range_whole:1; /* entire file */ diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 4fae912f7a86..3ee2058a2559 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -2360,8 +2360,8 @@ void tag_pages_for_writeback(struct address_space *mapping, } EXPORT_SYMBOL(tag_pages_for_writeback); -static struct folio *writeback_finish(struct address_space *mapping, - struct writeback_control *wbc, bool done) +static int writeback_iter_finish(struct address_space *mapping, + struct writeback_control *wbc) { folio_batch_release(&wbc->fbatch); @@ -2370,12 +2370,12 @@ static struct folio *writeback_finish(struct address_space *mapping, * wrap the index back to the start of the file for the next * time we are called. */ - if (wbc->range_cyclic && !done) + if (wbc->range_cyclic && !wbc->done) wbc->done_index = 0; if (wbc->range_cyclic || (wbc->range_whole && wbc->nr_to_write > 0)) mapping->writeback_index = wbc->done_index; - return NULL; + return wbc->err; } static struct folio *writeback_get_next(struct address_space *mapping, @@ -2434,19 +2434,16 @@ static struct folio *writeback_get_folio(struct address_space *mapping, { struct folio *folio; - for (;;) { - folio = writeback_get_next(mapping, wbc); - if (!folio) - return writeback_finish(mapping, wbc, false); + while ((folio = writeback_get_next(mapping, wbc)) != NULL) { wbc->done_index = folio->index; - folio_lock(folio); if (likely(should_writeback_folio(mapping, wbc, folio))) break; folio_unlock(folio); } - trace_wbc_writepage(wbc, inode_to_bdi(mapping->host)); + if (folio) + trace_wbc_writepage(wbc, inode_to_bdi(mapping->host)); return folio; } @@ -2466,6 +2463,7 @@ static struct folio *writeback_iter_init(struct address_space *mapping, tag_pages_for_writeback(mapping, wbc->index, wbc->end); wbc->done_index = wbc->index; + wbc->done = false; folio_batch_init(&wbc->fbatch); wbc->err = 0; @@ -2494,7 +2492,8 @@ static struct folio *writeback_iter_next(struct address_space *mapping, } else if (wbc->sync_mode != WB_SYNC_ALL) { wbc->err = error; wbc->done_index = folio->index + nr; - return writeback_finish(mapping, wbc, true); + wbc->done = true; + return NULL; } if (!wbc->err) wbc->err = error; @@ -2507,8 +2506,10 @@ static struct folio *writeback_iter_next(struct address_space *mapping, * 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); + if (wbc->nr_to_write <= 0 && wbc->sync_mode == WB_SYNC_NONE) { + wbc->done = true; + return NULL; + } return writeback_get_folio(mapping, wbc); } @@ -2557,7 +2558,7 @@ int write_cache_pages(struct address_space *mapping, error = writepage(folio, wbc, data); } - return wbc->err; + return writeback_iter_finish(mapping, wbc); } EXPORT_SYMBOL(write_cache_pages);