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 D6739EB64DC for ; Tue, 27 Jun 2023 11:16:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1DD2D8D0002; Tue, 27 Jun 2023 07:16:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 18D7E8D0001; Tue, 27 Jun 2023 07:16:39 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 07C9F8D0002; Tue, 27 Jun 2023 07:16:39 -0400 (EDT) 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 ED0338D0001 for ; Tue, 27 Jun 2023 07:16:38 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id BFAAB120188 for ; Tue, 27 Jun 2023 11:16:38 +0000 (UTC) X-FDA: 80948274876.08.9AF3A88 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf05.hostedemail.com (Postfix) with ESMTP id 1E0AD100002 for ; Tue, 27 Jun 2023 11:16:36 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=GrNr0wF+; dmarc=none; spf=none (imf05.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687864597; 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=q55zNrWcWX1seCjDV15l5s1oxcHr7EHzoCwODPWQFB8=; b=YR82BOz52Bgfa71RhD6xApDLProsGFe6iysiz5dnfwfaWKmv6LozBCJoRYdsgAW8/Q3jel UMm3cmUhl5pGxJavMntbFNHLPhTlExUqcFSVGl1oFUF8wijLfycX1b57n47ddrm6Pgv/9w rIkGGNRk79pbboa5LalTfylYLhupBKA= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=GrNr0wF+; dmarc=none; spf=none (imf05.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687864597; a=rsa-sha256; cv=none; b=vM9Y6vcsYJSZn3dZ3m+Yl8UrPULiiuFBC5ISGU+aTpZMGgnTvDpfmt/REvGpu4CyCwPiTF MKTb9s1pFp+fIh94m05zHo2G2RUW0/L6rKB/iSC4XqKgD2eFr7nyc6nKbDM1fWMx/PmxZ6 Tl8zEfsRPsjTNmL+QRX0J562vd1D0CQ= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=q55zNrWcWX1seCjDV15l5s1oxcHr7EHzoCwODPWQFB8=; b=GrNr0wF+UWMj9j/CemXePQbh2f UhyC+Z4kB+Q53ASus8GloxIp1c3MAarYTkW2nRn1swIualKPTLxdDCNqALlZBU3llcHyZQrf2vfIz 7yePy40bFfiPCAEngGlQqdCZq0UcizDiWEh15qjp/Rt8YjrnzQpR89Ob8z/udsYAVb1YBDG2izgu3 KxhBTGtRSTRLdJlVoKNWU03M6MdgzokM2XoHTIITgQk2Lz0L8PM7rwydCgkKBkn/nOF95YZKBGvtr QlSzZiRwHKadmIku2zVxtEj+AMiO3hHut6DtYkic76BoD2BulNflsSdrlTghATY6aNuvEQrUglZkY OYojYUww==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1qE6gU-002egV-Sn; Tue, 27 Jun 2023 11:16:34 +0000 Date: Tue, 27 Jun 2023 12:16:34 +0100 From: Matthew Wilcox To: Christoph Hellwig Cc: linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Jan Kara , David Howells Subject: Re: [PATCH 03/12] writeback: Factor should_writeback_folio() out of write_cache_pages() Message-ID: References: <20230626173521.459345-1-willy@infradead.org> <20230626173521.459345-4-willy@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 1E0AD100002 X-Stat-Signature: zhwb39yyjcbx3zfjbnc4mjub1myizd4m X-HE-Tag: 1687864596-693439 X-HE-Meta: U2FsdGVkX1+zRwld/h8RlfKNQnOAnhzUA2wNiyim+sc4uwBZu+/brMemk8Ntb+CfDND5xTa6xxAu3VYAh8H9AAx5DToFwJj3Y5TeLGqSBGFzRrKbRGx0fsMuyo4/fBJ2vgwTGjLangOaZg1wod0g6lHH/QQnql7eBwaC+qEMOtzL/IZqK/GeoQsFRrIk0BVA2DMXsF/rdi0HDW3MmGrllRLNJMdAsB2ZJ0bN+P60ucijBHpPQtftLRn/URGx3sxvEwVmo5UJEBOrdUW8QpHizTORxeJ8S4XTxM7ijFn1Y6HQhhmsjVPlhi1KM/AuLLHsu8AKp4ctlTcTipJoGdUu0HwPxrKCFnB0UBA2h7L9o+7DFhKEe56fKyh1P57m9Cc/tldUJiEi2id3xcYzVi/f/M6ih5MlAH8T84bvBVjRA7JSX1vltkeHa2qd2GQnmnre96ajBgCnHaGGWmHcCVXLjXhc+dm8PXCDnjLbbWFe0IiSLhijNpEfPisJYN3vY6Lz+zOnsKnw3svco3r2NuMyEs15qYcgDoGiKiHPnzSbTJcwU6gbzk6gNM+2UiCwCZSQ31T/SMgKRaJmxxuf3GYi1ZePjH2fki/HQGdxLRgZqFXXVian2pzeisYl4o7uVMMLggEe5bxaijaZwcrd1c+8W9m5coTwSNJNQ95dop2lUpDeQffu06s5D1WwAjqO2IXzegf2zV94KmWe2n+HEDzymgjebfuZvGpAGfoNoEeeZFTKbKjIiWzZSWR65lyouJkZq7Ut1WZ+aL0tINeHSUClI66AGCayORfrLh80j+sMyyvmHppd0AXgsG8m5t8jnQWYn8JogUS7foTQFg1vdFB3eI04OmC+h/g/ky5wUoCr32/XJnuO+4KzEnCuX0MJlU0bIrsW62Bu5w5L+KoqRX6L4Xpx9o305uyja+uDJjuFQfkXnvIwOeDFAvsAmHLwJoGFMuG1R4E93l9AT3ecBqj 55DAF2Lv 9oTZZzPxBWJ1eCuHjmhWfyJe4QrxxCtKzzDwh5bEmE8bo5wzcsIF0t36+Z/Vky2Pz8sxFi5836jPVj9nV1zwgLyJjFbCsPhH174wqZg06Y5O2VoLhXSvPTKgfgNBZpdNCJ0gQ8Cvba+7H8NbOL1/MPi0TEPYsJxHTEXDHoTx+GVaR1YoOfZYHxhnfAA== 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: On Mon, Jun 26, 2023 at 09:12:07PM -0700, Christoph Hellwig wrote: > > + if (folio_test_writeback(folio)) { > > + if (wbc->sync_mode != WB_SYNC_NONE) > > + folio_wait_writeback(folio); > > + else > > + return false; > > + } > > Please reorder this to avoid the else and return earlier while you're > at it: > > if (folio_test_writeback(folio)) { > if (wbc->sync_mode == WB_SYNC_NONE) > return false; > folio_wait_writeback(folio); > } Sure, that makes sense. > (that's what actually got me started on my little cleanup spree while > checking some details of the writeback waiting..) This might be a good point to share that I'm considering (eventually) not taking the folio lock here. My plan looks something like this (not fully baked): truncation (and similar) paths currently lock the folio, They would both lock the folio _and_ claim that they were doing writeback on the folio. Filesystems would receive the folio from the writeback iterator with the writeback flag already set. This allows, eg, folio mapping/unmapping to take place completely independent of writeback. That seems like a good thing; I can't see why the two should be connected. > > + BUG_ON(folio_test_writeback(folio)); > > + if (!folio_clear_dirty_for_io(folio)) > > + return false; > > + > > + return true; > > .. > > return folio_clear_dirty_for_io(folio); > > ? I did consider that, but there's a nice symmetry to the code the way it's currently written, and that took precedence in my mind over "fewer lines of code". There's nothing intrinsic about folio_clear_dirty_for_io() being the last condition to be checked (is there? We have to redirty_for_io if we decide to not start writeback), so it seemed to make sense to leave space to add more conditions.