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 1D3D2C77B61 for ; Tue, 25 Apr 2023 03:23:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6F6DB6B0071; Mon, 24 Apr 2023 23:23:34 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6A6BF6B0074; Mon, 24 Apr 2023 23:23:34 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 56E746B0075; Mon, 24 Apr 2023 23:23:34 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 448C96B0071 for ; Mon, 24 Apr 2023 23:23:34 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 15C53A032E for ; Tue, 25 Apr 2023 03:23:34 +0000 (UTC) X-FDA: 80718468348.30.7EBA26E Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf17.hostedemail.com (Postfix) with ESMTP id D592740004 for ; Tue, 25 Apr 2023 03:23:31 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="GefC/jvF"; spf=none (imf17.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682393012; 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=TDZKORwzEt+eX2u0tCokEp9eJLCllrYXgwK+Hx7Vcpk=; b=1q7M3Pj9abMub3PqKwwliQ1y0DuEFPKuqOB5ymp1Sz2IdK/4J39cncQ4d3y2X544/Hfb4e /48ROcXofXwqz32FlwbXiQOtH/+Qsi6xW/FBNG0kX8GCc5pOe3l7cYmOJi6w7CnNI6cca6 /KEdcTyhP6mb9NHUjPrShbKTV3bVRGI= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="GefC/jvF"; spf=none (imf17.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682393012; a=rsa-sha256; cv=none; b=0DqUacqvYqNi/Ocv0EhFRRLncBjOUqFAdUdwLJ8icJO844zusX7GTex68ZkbBEF+AOz2MA t5wxireJ5TQFUhJMpnLoHplfHv6g3L1JkV7WJRVaLNNtTfk/fegUOy1v/h9wjSpZTnARcV WGtA1uO8K8+EnVHC8koC34A+CsSUT8U= 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=TDZKORwzEt+eX2u0tCokEp9eJLCllrYXgwK+Hx7Vcpk=; b=GefC/jvFTqeaTIMWQN69ufb2Me sAIXG1NtP1yjyItvIAQpV0D4kpWWMhKNhQ1j7uhKMIIODYwXfUda7IdDsaBIhpXIsyyQw26Sif0xU GJZGrm/HqpFBiMyB/OCvROv64NNHzEONcn4r9rYmrzYE8JEe1lhDl64E/NbQHEVVgLEYVLDf7XKAi 0SpwJfGx2NfwRrdjwnDsO45GmhdvYbJzPdco38YI9bjf7cE5XFGQ42/LX9kwn2tJodQ+6PsDqbShV 5rAe0q8HQVGBAIm743Vmp0GETWCgQXfu4wG2x7rWLxS4ZB1Xz+HQWvzswUlMKnTru3B82Q7Q7VY0C FT4X0aWw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pr9Gx-0014cq-LB; Tue, 25 Apr 2023 03:23:19 +0000 Date: Tue, 25 Apr 2023 04:23:19 +0100 From: Matthew Wilcox To: Andrew Morton Cc: Yajun Deng , david@redhat.com, osalvador@suse.de, gregkh@linuxfoundation.org, rafael@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH] mmzone: Introduce for_each_populated_zone_pgdat() Message-ID: References: <20230424030756.1795926-1-yajun.deng@linux.dev> <20230424145823.b8e8435dd3242614371be6d5@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230424145823.b8e8435dd3242614371be6d5@linux-foundation.org> X-Rspamd-Queue-Id: D592740004 X-Stat-Signature: 5b41dd743ycsos33tc83fpdqe4dhz6jr X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1682393011-89420 X-HE-Meta: U2FsdGVkX1/8Y1eMXTbk/1UXUTbCTBBSeg7WCSFl3iLFnhgf7DU8dmPODJFLyI9JQRL4lROXJVAH1xkccSDNC3omcWOt3yJDXsjf4o7zKM3+G1CL+zm73wEqGuE+AjoH7X09efTumgDZcB5nKpnvfYQoT1T1Y14iXzu1CYw0uioYlW60RZRv/MoK8V7WD6ICVmZsP91nsd2d2eWG9RDtrlTeb2+LSIUQGbNH7LCBGsimJCfjBmpKfiZPI2qz7YQElj2GFtS+73bARY5Yx1EVliz16BtvtIUcGbCCdmEj+736H9dvPk2gzRipXsJtKe57z0Zn0U/ymsufZf/1zaYB8YLLESK2NT8ARRqkTubI7QcPEA1rI8bCiawWUf3mE1D4dkRg/KAFNUoyOa8fX2ZPDSrzwDi+oFrrKY0z7m0uzHVdINXLamTDnWm5mrIJr+5lz1h71yYV67SeaNbtbVmgyQ5v0KuI/+iKieYPJuQQUhqnWiqXhbDGdhjfjLk/pzSKrw0l+4UFeZ5+0Oife1Ke8z+VKSsLyIdsGHHTFmXKmJuW4pVmyfvFRWs81/0li/DwjwHcHazyWV84UKtdG9uMXimp2h35jXchDi8/W7R+S22FDFW5tgGp8WL6+9mhm6d0TMko2T6ZU4P7Ur4CfeP9oUd3MbvGC6AHa6cu3su864V8ozXIEF0XAdRnZsyuPow5C4Na7bCXwoJJD2P+YK/Um96cGCEykAU3crsTAOUfKfHL/hqGnqyPrfPMBL/k6rPAzcW3mSL8EY50mfUwP97epNpKsi/1kE2uTO2SHfxHge6CsjkUkyfiql3hzyIzRrSxaq0jq8zr3Vd9dN9VTXn03chPKaeUXpHLR5/iP6Wd1/Wf2uaVBcb/Hw2VSuSuqKrauaGx2seqB35zXb9qLYzamB/JNo8oltrjc/ig2Xtcfh7fQzVplmE4ROv99WIhKDfEgzxt3J8/LhmMDrbt04i xAw60OWo MrY3KiDsHh+KS3uX0ra8vXtW+U6bhvaiS9bV4cV8p/zRsm2qvsWpCLSS7Tgy3ZQV2FZTC2kHQ8fJOmSDm3ZsDMex1nY71u5/CNmG0+9HSp5EgcPwzxfowRy02lFe1yLf88CvYFd7Ay+yqZ/cLIEZeAXJRufD4xjn3ypAqDUjVHVdrsXcE4XydZXGdXGRzmvcz8sqFvdtR3woKCAtAvboZTF1OyJh0qi8GF4Lvzbh+pJfK066sED1tOcCYSQ== 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, Apr 24, 2023 at 02:58:23PM -0700, Andrew Morton wrote: > On Mon, 24 Apr 2023 04:50:37 +0100 Matthew Wilcox wrote: > > > On Mon, Apr 24, 2023 at 11:07:56AM +0800, Yajun Deng wrote: > > > Instead of define an index and determining if the zone has memory, > > > introduce for_each_populated_zone_pgdat() helper that can be used > > > to iterate over each populated zone in pgdat, and convert the most > > > obvious users to it. > > > > I don't think the complexity of the helper justifies the simplification > > of the users. > > Are you sure? > > > > +++ b/include/linux/mmzone.h > > > @@ -1580,6 +1580,14 @@ extern struct zone *next_zone(struct zone *zone); > > > ; /* do nothing */ \ > > > else > > > > > > +#define for_each_populated_zone_pgdat(zone, pgdat, max) \ > > > + for (zone = pgdat->node_zones; \ > > > + zone < pgdat->node_zones + max; \ > > > + zone++) \ > > > + if (!populated_zone(zone)) \ > > > + ; /* do nothing */ \ > > > + else > > > + > > But each of the call sites is doing this, so at least the complexity is > now seen in only one place. But they're not doing _that_. They're doing something normal and obvious like: for (zone = pgdat->node_zones; zone < pgdat->node_zones + max; zone++) { if (!populated_zone(zone) continue; ... } which clearly does what it's supposed to. But with this patch, there's macro expansion involved, and it's not a nice simple macro, it has a loop _and_ an if-condition, and there's an else, and now I have to think hard about whether flow control is going to do the right thing if the body of the loop isn't simple. > btw, do we need to do the test that way? Why won't this work? > > #define for_each_populated_zone_pgdat(zone, pgdat, max) \ > for (zone = pgdat->node_zones; \ > zone < pgdat->node_zones + max; \ > zone++) \ > if (populated_zone(zone)) I think it will work, except that this is now legal: for_each_populated_zone_pgdat(zone, pgdat, 3) else i++; and really, I think that demonstrates why we don't want macros that are that darn clever.