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 1388CD2444F for ; Fri, 11 Oct 2024 01:10:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 900666B0083; Thu, 10 Oct 2024 21:10:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8B06E6B0088; Thu, 10 Oct 2024 21:10:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7779A6B0089; Thu, 10 Oct 2024 21:10:18 -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 5B0946B0083 for ; Thu, 10 Oct 2024 21:10:18 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 405F0AC0A3 for ; Fri, 11 Oct 2024 01:10:10 +0000 (UTC) X-FDA: 82659540474.04.1D5D43B Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) by imf05.hostedemail.com (Postfix) with ESMTP id 2ED89100009 for ; Fri, 11 Oct 2024 01:10:11 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=G7ZCQoeE; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf05.hostedemail.com: domain of ying.huang@intel.com designates 192.198.163.13 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728608864; 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=LACxjZNorlg1EdmLITGxuYp4vFzPwKy+Sqb3tlcTB9A=; b=onWw0A0i2iIm5SCkDzanpDQ5Ok3QYXmoozdnEH6xmKvTqqZwXXvMOuCrLZdHzldA+zursO MZ4NxsNitLC/aZDI1qiXpAR27nocMFHbt+ukBRqOfRV1bhTRD9DzUVE/38ryXPwxwuFnb0 SOhdImuNM/ZMiT2Df01hPNH4/Hc9VJY= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728608864; a=rsa-sha256; cv=none; b=k4fkCJIMYx+CUbLSisKiWVwJtWegni425/n/ob8d1vQ2WYMyR7m2C+uO8zj6YoIQSKIkU1 UsJWCc+mPCQxhj9a4VeSOuCR0ZmMRNgYLd11GP1t8EhW4O5FUA3dO0xZUTpZ8ub2BCmF2k FKUvxb1nu5EmyenLl6dx8e14uPFxhqw= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=G7ZCQoeE; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf05.hostedemail.com: domain of ying.huang@intel.com designates 192.198.163.13 as permitted sender) smtp.mailfrom=ying.huang@intel.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1728609016; x=1760145016; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=CxcNe39wol/3RMMdthb7jSSJhVKt++1yY7sFwySt0Ug=; b=G7ZCQoeEKu1ur8J9EVf3cr1w3ZPAqCn5bc3p+CxoKgtaL8eplkr2Eltv OxzYP9727FJq/SQJl8T8BvbztfKDdO0E9QYw8dg2jLnDR/0KdE3o7lSOg xrW0WGDmJfyEOkkVH4gq+Y8OaTSVQdbEaKV01/Z+cbr84SqWCgc9RL+eF DEb7+gZlUpRWWWPr253/4vxblGLLPenjxjSe/7Y0LxqOJITpy3/4QmfBd qLN7Ehes05osLYdSdN8OEz9OT4azw2I55vz8oSZc/xdpy/tZPbbcGQWqz mpP/RX5rWXMp0ZFhYs73ezn44hPsskkP+fGL6mnaJ0cdKZKm9ogNHAYoL w==; X-CSE-ConnectionGUID: 9JHZTaeRSKClnSfi6wpPdg== X-CSE-MsgGUID: kwq/xezcTdqDsBmvLZa/wA== X-IronPort-AV: E=McAfee;i="6700,10204,11221"; a="30869211" X-IronPort-AV: E=Sophos;i="6.11,194,1725346800"; d="scan'208";a="30869211" Received: from orviesa002.jf.intel.com ([10.64.159.142]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Oct 2024 18:10:14 -0700 X-CSE-ConnectionGUID: cLQw4wNyTAeywHSh/b7aKQ== X-CSE-MsgGUID: FkIIDcY1SSeg17kKRAtytA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,194,1725346800"; d="scan'208";a="107629082" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orviesa002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Oct 2024 18:10:11 -0700 From: "Huang, Ying" To: David Hildenbrand Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org, Dan Williams , Davidlohr Bueso , Jonathan Cameron , Alistair Popple , Andy Shevchenko , Bjorn Helgaas , Baoquan He , Dave Jiang , Alison Schofield Subject: Re: [RFC] resource: Avoid unnecessary resource tree walking in __region_intersects() In-Reply-To: (David Hildenbrand's message of "Thu, 10 Oct 2024 14:54:33 +0200") References: <20241010065558.1347018-1-ying.huang@intel.com> Date: Fri, 11 Oct 2024 09:06:37 +0800 Message-ID: <87set3a1nm.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 2ED89100009 X-Stat-Signature: yxo3gh1ntzuhrzh8t3yby7j95xzfytqq X-Rspam-User: X-HE-Tag: 1728609011-61717 X-HE-Meta: U2FsdGVkX18nD04tV37B/37ZsEoenhBvlKM1C5dP3zw4zQ7jNg0ik3A+/r1hlSmf37/M/bzmwSVMxYlMYHoml1TPTzCqzOj2OFl7M5MZfbC06zr82xLEJHk2TksOKbngrtoq6gqcXbFVmwsj9RrnLWpmg9BirvtJIQ40+jD9QN+JPa8sozMO22X/WLprP9N7xkKYmA6Kz6QQ9w9gKPhdtlLkyBq3DacprTliWlhyHKRJEnF/H3L0bCdeNx6pD2xeTp5645YjX2G0E4R9wC0gRaX3XECfU1Y4ZYIIzNpWran2Ok6Cv6ZMDyVYtW0/xG74KVGbVXLH2/CvGnJD9rRNYJYvQ+WhGFzX7TY0Sn1U0FUtPMZM8a9zFKtww5Hey3bSG8RrYaBEOSt/4qqpMYW4rd0tCcvMmqaKHZrGRBqymdGWLmGSClOF57yzMW7Qq0GwEUZyJqshheS84FXNKWEuCN/t35F78wKKlagoNhhgN6j4pdkmAVbpHiday4Ncn2dkNkq93FI4FeyJYyohyLHlzYE9N6eqVSTzPVaXYBTzXkJ0EjVQJWNBRBj4SA9K7kjbb7fV/MFPKBqzlyWpB9pz+IIqFVw2Izluikq1iXWIfPLtRS9O7uZaNRH4N5L+89bXdu4wHJm4iR35RA6bGFH6/SPhZ5a7h1L9d4+fYVWsZ5G5WX4qWTzoZ3itTgEyh/qh8SE55mXpsgbfK0BFTv5HdfkpxXFAhvpliwTsPmGxPOrT46c0+0IISbfsH2Hck4IqPmFbh9K2RcjOQiw4GbdwyUObxG7QLqknhVoIhUMA5lw1ZTa5ijmjIfRZWPkfvihfkPS1rS2GgSr3HEFSE5zcXYKNwwU3/eDOnycmTTMKNULR5x3D1oUZwMnjxphK4G1D7QciFTDqgSWNB2Z7ujaz9XFTPkhilerG1SjQsYsQHOqWhM2ldG91sIeONPTbYFcIe4BshEjoBpOV6kopDz1 97Aj1ASA 2JpCP4arkOdL10LKCtm/ZOvJ3699ypWS/lTNU7TDHjlW+A5/vOMMfCh+BAKBGIWfA3gk6WVvN4NuKBnQdDXrl7xykSvliPU5yLAY8V/EEcTgr/m5Nv3C6SPgz6e9b+NDE+ckhcTScdbzHyxnJh9Ap5zb3NAnk1Dr+OMHas4ffruYJ8RbNMyPC7M09+6dEpJOP4ySZ+79V7tJ0Ow5nbSTQ4PQ7SrUpaO47xuPraugIlsKnw6WgNoaHMsI8OQ== 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: David Hildenbrand writes: > On 10.10.24 08:55, Huang Ying wrote: >> Currently, if __region_intersects() finds any overlapped but unmatched >> resource, it walks the descendant resource tree to check for >> overlapped and matched descendant resources. This is achieved using >> for_each_resource(), which iterates not only the descent tree, but >> also subsequent sibling trees in certain scenarios. While this >> doesn't introduce bugs, it makes code hard to be understood and >> potentially inefficient. >> So, the patch renames next_resource() to __next_resource() and >> modified it to return NULL after traversing all descent resources. >> Test shows that this avoids unnecessary resource tree walking in >> __region_intersects(). >> It appears even better to revise for_each_resource() to traverse the >> descendant resource tree of "_root" only. But that will cause "_root" >> to be evaluated twice, which I don't find a good way to eliminate. > > I'm not sure I'm enjoying below code, it makes it harder for me to > understand what's happening. > > I'm also not 100% sure why "p" becomes "root" and "dp" becomes "p" when > calling the function :) Likely this works as intended, but it's confusing > (IOW, bad naming, especially for dp). > > > I think you should just leave next_resource() alone and rather add > a new function that doesn't conditionally consume NULL pointers > (and also no skip_children because you're passing false either way). > > static struct resource *next_resource_XXX(struct resource *root, > struct resource *p) > { > while (!p->sibling && p->parent) { > p = p->parent; > if (p == root) > return NULL; > } > return p->sibling; > } > > Maybe even better, add a new for_each_resource() macro that expresses the intended semantics. > > #define for_each_resource_XXX(_root, _p) \ > 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. 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)) > XXX TBD > > Or do you think this should not only be "improved" for the __region_intersects() use case > but for all for_each_resource() users? I cannot tell easily. I prefer to make for_each_resource() to traverse only descendant resource tree of "_root". This helps code reusing and make the interface easier to be understood. The difficulty lies in twice evaluation as above. -- Best Regards, Huang, Ying