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 5AA7ECE8E71 for ; Thu, 24 Oct 2024 13:02:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E337A6B0089; Thu, 24 Oct 2024 09:02:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DE30B6B008A; Thu, 24 Oct 2024 09:02:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C83406B008C; Thu, 24 Oct 2024 09:02:03 -0400 (EDT) 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 A8F516B0089 for ; Thu, 24 Oct 2024 09:02:03 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id AD8B8140D7A for ; Thu, 24 Oct 2024 13:01:43 +0000 (UTC) X-FDA: 82708508064.15.499CFCB Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.17]) by imf08.hostedemail.com (Postfix) with ESMTP id ECCF916001F for ; Thu, 24 Oct 2024 13:01:47 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=CY16NWev; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf08.hostedemail.com: domain of andriy.shevchenko@linux.intel.com has no SPF policy when checking 192.198.163.17) smtp.mailfrom=andriy.shevchenko@linux.intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729774752; 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=HfAhYPYLPEwZl64BKyZW9lI64xpq11a/a4pZTM/ysMU=; b=Qt+xwzA6yWU3Ffwf9M+kcPF6LQVyKll0kfKk/GorTPv8NlRHC0YNO38O0YD9syAm4Ojv1a zCsffwYZB5BrOlspbjEG1cBQ6V5p3l3HBBarS24/FVfM7EJ5dftIyGuTnBo+1p5UqhHW2i X17CvZP5kmJIZEANCAEboMT0DQ2WCto= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729774752; a=rsa-sha256; cv=none; b=UF13MyzRtPGOPRIt9j8Aq/oL5LUElzORQ/fTGV52joTOzwYTagd+gmgetzzWCFjmnweMrE NkDlsdTw+lLlrLZZIlVCMb2q/5LDgCvjaD6WC4ypqLHrI10nUXGmT2YjkagcdY5xr74IYZ ibFRKlpjK7Wztev2rfx2vMCnPkGoQRg= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=CY16NWev; dmarc=pass (policy=none) header.from=intel.com; spf=none (imf08.hostedemail.com: domain of andriy.shevchenko@linux.intel.com has no SPF policy when checking 192.198.163.17) smtp.mailfrom=andriy.shevchenko@linux.intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1729774920; x=1761310920; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=2a+9bKMv7hVzknlW8OazkLUPwrbxo5rblBY4AWBW2pI=; b=CY16NWevZLqXhEQXvK2bSE8cctLfnDvkuCnioOYL1v5/fD3YDiqwtqav aau4N+yyKYP3cin+80GOghhIUEsC5f2jp5wAwWkg7jfzQkqPLb6d58gXV ZJLA+FKaeTBWWE/m/XcPV2zmW1GN+xVsh9+FQuwgMcaUV6Fkk6G1RIwY5 9g9uqsl/WzWJsoMSo+2urQpUwRSksW3XNcfPFFXrbOYoGhoJ2LU5yFLhc Ec/StMBCCyhDidJJNTfvSzsduWDf1URsGgpJ44D5AQV11RN4toTRBh0z/ MC+mcoSZZ7fyQM0uHiTiuoRXRhvHp50K2LcILnj+aoMrQMx8xZ8G6kosC Q==; X-CSE-ConnectionGUID: PryykTFHQUqvic8qqIx3bg== X-CSE-MsgGUID: uf1oCLViQt+WWQQlYkgSkQ== X-IronPort-AV: E=McAfee;i="6700,10204,11235"; a="29303309" X-IronPort-AV: E=Sophos;i="6.11,229,1725346800"; d="scan'208";a="29303309" Received: from fmviesa004.fm.intel.com ([10.60.135.144]) by fmvoesa111.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Oct 2024 06:01:59 -0700 X-CSE-ConnectionGUID: Cd4N0AKgRuanQXpDlER8KQ== X-CSE-MsgGUID: BgcMOECEQKqvDTOvgmvjfQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,229,1725346800"; d="scan'208";a="85191643" Received: from smile.fi.intel.com ([10.237.72.154]) by fmviesa004.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Oct 2024 06:01:56 -0700 Received: from andy by smile.fi.intel.com with local (Exim 4.98) (envelope-from ) id 1t3xTM-00000006Zui-3IcU; Thu, 24 Oct 2024 16:01:52 +0300 Date: Thu, 24 Oct 2024 16:01:52 +0300 From: Andy Shevchenko To: "Huang, Ying" Cc: Dan Williams , David Hildenbrand , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org, Davidlohr Bueso , Jonathan Cameron , Alistair Popple , Bjorn Helgaas , Baoquan He , Dave Jiang , Alison Schofield Subject: Re: [RFC] resource: Avoid unnecessary resource tree walking in __region_intersects() Message-ID: References: <20241010065558.1347018-1-ying.huang@intel.com> <87set3a1nm.fsf@yhuang6-desk2.ccr.corp.intel.com> <671965a8b37a2_1bbc629489@dwillia2-xfh.jf.intel.com.notmuch> <87wmhx3cpc.fsf@yhuang6-desk2.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87wmhx3cpc.fsf@yhuang6-desk2.ccr.corp.intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: ECCF916001F X-Stat-Signature: 3x5wi6oboy8jx5pkxrbpfefjirmefsft X-Rspam-User: X-HE-Tag: 1729774907-177569 X-HE-Meta: U2FsdGVkX18lxyrzmSJrIHd3MEiqRqmszr0/amNfiH8339FqB+1WkxoWK/nz6p7ovu7+rR925QYzyX3sxyPGoJIMz+U7XAhHCkpUiaAbG/mlFwzR/M9ZUbqn/h/nWTWEIe1gf4XV1+oAp481chTqAIT+HSXYDqVnMy84TMJFiLMIGRpOZcrGqQWhSVJkIQmIHGGGJZ1idrkdvBoStCH7o1z/Rp8zAh5Uh5P4khS6qqh89rpRCLN6rqnrzljDJN7P+5AA4e3x2yFSPLuEdCFy5Qe2KD94kjrt+tmSg52DDktY2ZG1EKrCYtYw6pGLIfzG+5sGYG4rrvOef45wCiWKAziK0+IqETbLA/N1whXOqejxCzR0rKdYNLPCmk3HmnA2uedvQ3/u+cBbFc+ZAeSepux/pbWaKm2tXQx20fJFwjFbURM9E1BnACAmhTIeHPX25s1weZ8U+xckxvq7JaNDI3QWOq4Y5XazBsxoHZhl23yyZVNYvuhIb5tEQwSdamE/KqW3BWPpvFmhYP58ntUP00ka10JLNZggr9HGExTagS/vWCTlfLx2h+HDh8+nFqw32A3Tva9NL/jvUIkBBgmincR2+MP+lPhBdu35cjPjsYXE+no1wihnSlfkT3Z7D2hp9QRj5J+WDhpnQQSf8YZVTMp7ZB5/XYRTO9+Lf8OGoGi2dLXlxvIH4YX+OSLRufnQMDmWbRwrNItJYUBLvrQxWPYhFtXWHTKJJzoRxUXmWsDI51VOafGuQUpOXXIur+4p6z9JGdLckJkWBAtmX3nNVCd3e62dks9qBSjfh4lSK0xsgBTmvp5O1oEVB+sfsXKNg9B7HxT0ZJdoyjw6nLBxwqCEWn7uwtxHK7+HHVHOS7NkTISXAM1GWSMBhdaYcQadv8E/t0ceJB2BpUGVXDJ81Vy8gjDuSaJMfMqd+4DsSQGxyknW3bCRTHdMCTYYzWLh+r9gK1zAZH31NP8AV16 d+B6xq6H WrUUkiPLs9p+pm2cZhcyd99K7CCrD30BsiOst2DILEIlfBkgaMWhR1f8YBkV2m1bSqswzByhz7Dzu3+GzJHWpiJ6X47QmgDLr4UIbEiRElT8YsHttsMTo8rEoY1ad4vQtRIhmRTG2pK2i794WbyLi58EOKAhfKeQpMGjiqh27qNue5+HbGyRxMwN1YY3DWCYCB5GYo/3MQUiZPpU6MQ4XqgORxbfehQzlxChZ2pPlCOFpduwh+AkfKjXyiTzJwLyg9kDgQ2835wMXRpgV56Yp0qrlw/p43DYIwmV9a3d1FzG6FYM3PCTEEwYkSgt9rOqHyGJI 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, Oct 24, 2024 at 08:30:39PM +0800, Huang, Ying wrote: > Andy Shevchenko writes: > > On Wed, Oct 23, 2024 at 02:07:52PM -0700, Dan Williams wrote: > >> Andy Shevchenko wrote: > >> > On Fri, Oct 11, 2024 at 09:06:37AM +0800, Huang, Ying wrote: > >> > > David Hildenbrand writes: > >> > > > On 10.10.24 08:55, Huang Ying wrote: ... > >> > > > for ((_p) = (_root)->child; (_p); (_p) = next_resource_XXX(_root, _p)) > >> > > > >> > > Yes. This can improve code readability. > >> > > > >> > > A possible issue is that "_root" will be evaluated twice in above macro > >> > > definition. IMO, this should be avoided. > >> > > >> > Ideally, yes. But how many for_each type of macros you see that really try hard > >> > to achieve that? I believe we shouldn't worry right now about this and rely on > >> > the fact that root is the given variable. Or do you have an example of what you > >> > suggested in the other reply, i.e. where it's an evaluation of the heavy call? > >> > > >> > > Do you have some idea about > >> > > how to do that? Something like below? > >> > > > >> > > #define for_each_resource_XXX(_root, _p) \ > >> > > for (typeof(_root) __root = (_root), __p = (_p) = (__root)->child; \ > >> > > __p && (_p); (_p) = next_resource_XXX(__root, _p)) > >> > > >> > This is a bit ugly :-( I would avoid ugliness as long as we have no problem to > >> > solve (see above). > >> > >> Using a local defined variable to avoid double evaluation is standard > >> practice. I do not understand "avoid ugliness as long as we have no problem to > >> solve", the problem to solve will be if someone accidentally does > >> something like "for_each_resource_descendant(root++, res)". *That* will > >> be a problem when someone finally realizes that the macro is hiding a > >> double evaluation. > > > > Can you explain, why do we need __p and how can we get rid of that? > > I understand the part of the local variable for root. > > If don't use '__p', the macro becomes > > #define for_each_resource_XXX(_root, _p) \ > for (typeof(_root) __root = (_root), (_p) = (__root)->child; \ > (_p); (_p) = next_resource_XXX(__root, _p)) > > Where, '_p' must be a variable name, and it will be a new variable > inside for loop and mask the variable with same name outside of macro. > IIUC, this breaks the macro convention in kernel and has subtle variable > masking semantics. Yep. In property.h nobody cares about evaluation which makes the macro as simple as #define for_each_resource_XXX(_root, _p) \ for (_p = next_resource_XXX(__root, NULL); _p; \ _p = next_resource_XXX(__root, _p)) (Dan, that's what I called to avoid solving issues we don't have and most likely will never have.) but if you want to stick with your variant some improvements can be done: #define for_each_resource_XXX(_root, _p) \ for (typeof(_root) __root = (_root), __p = _p = __root->child; \ __p && _p; _p = next_resource_XXX(__root, _p)) 1) no need to have local variable in parentheses; 2) no need to have iterator in parentheses, otherwise it would be crazy code that has put something really wrong there and still expect the thing to work. > >> So no, this proposal is not "ugly", it is a best practice. See the > >> definition of min_not_zero() for example. > > > > I know that there are a lot of macros that look uglier that this one. -- With Best Regards, Andy Shevchenko