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 8270ECFD2EB for ; Fri, 11 Oct 2024 08:52:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B6BDB6B00A8; Fri, 11 Oct 2024 04:52:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B1B336B00A9; Fri, 11 Oct 2024 04:52:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9E43E6B00AA; Fri, 11 Oct 2024 04:52:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 7DC336B00A8 for ; Fri, 11 Oct 2024 04:52:06 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 910281A0F0D for ; Fri, 11 Oct 2024 08:51:58 +0000 (UTC) X-FDA: 82660704210.28.463B1AA Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.15]) by imf18.hostedemail.com (Postfix) with ESMTP id 8416B1C0014 for ; Fri, 11 Oct 2024 08:52:02 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=YoIHPwXa; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf18.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.15 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728636695; a=rsa-sha256; cv=none; b=1Kuce9Bvdgf0BqARY/sQ3z/OAgUwCprwZdIirkjwrgEjnPZY/KsR3hmuCyfrSobs0k2ujG 54jRBK8MyHWjDFMkhrVcBcMbZqnOljRzJ4LbtU9NwcATdq1ZCNtq7Wa4mqpwPh4ovgJDBP uoDT46hLC3Z4OoLRelF3FvDZ2nj+a/4= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=YoIHPwXa; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf18.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.15 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=1728636695; 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=izZudiq5l2gP5BSdlO10fCw27zyDLug/f9zke7qHM3M=; b=Uxw0nSkymgkTO9mZXHryMr1OJWhX4SocWVPJfLBmjbbwStdTHVKIx28WEV0ZvnEpWFOuSL 5HDKX1AYCtSlbz+y7/WDDn1uO02VWxmtBh32b4mzblVIjhzfTG/hMcf+QQ/r2uklK+DJfj 4eqizHcQxHN7vLpLfCmUxmQw+ym9sdE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1728636724; x=1760172724; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=uMFBtSJwpjFyt4XPOYTWRjCnEGz3qayb9sH9HP8Kf+A=; b=YoIHPwXaAurVvKu0f/obKjIntz3NcrsChpFFHwDmNcik7ULA8KADnkLY i3S3ZDulogPR5IWIIatav5m7+L/9jejqp+bLfQgOEFpBnWrK6/9rcvZ+T Z/dyqNFfkUqzBNnvoDzholJkyh4/+wo82Tb3km6KtanDzw9ZGLoA2cWXM KqXTfA7xSTZtYCfI/olMeroeiQEz76xGU7PUq7FS2YSTVm6OAEsuRulRG cou3CwQr9GgYGbopaxK0wCf3MJv9rckgt8uH//seb8ZUtHAKQSKygG5mn IYxR6jAY+Y3aSBVJo9ifUkD2JNDZDSM98Y5HniOJIZWTJqA+YweYlyP5p Q==; X-CSE-ConnectionGUID: OfT55zTNQBehmFyr79CrcQ== X-CSE-MsgGUID: M2ZptfEDQuGsVeb3RZE6DA== X-IronPort-AV: E=McAfee;i="6700,10204,11221"; a="31729568" X-IronPort-AV: E=Sophos;i="6.11,195,1725346800"; d="scan'208";a="31729568" Received: from fmviesa003.fm.intel.com ([10.60.135.143]) by orvoesa107.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Oct 2024 01:52:02 -0700 X-CSE-ConnectionGUID: YW2FBySGQKSYW32O6JD87g== X-CSE-MsgGUID: PKap7fe8RumhHfbm5A0wqw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,195,1725346800"; d="scan'208";a="80854591" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by fmviesa003-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Oct 2024 01:51:58 -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: <38566dbf-1293-4fd5-9cbd-385e6c35344c@redhat.com> (David Hildenbrand's message of "Fri, 11 Oct 2024 10:02:43 +0200") References: <20241010065558.1347018-1-ying.huang@intel.com> <87set3a1nm.fsf@yhuang6-desk2.ccr.corp.intel.com> <38566dbf-1293-4fd5-9cbd-385e6c35344c@redhat.com> Date: Fri, 11 Oct 2024 16:48:25 +0800 Message-ID: <87zfnb81pi.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-Rspam-User: X-Stat-Signature: ym1s7y55twhr1hbaynsrebmnrhm81rk7 X-Rspamd-Queue-Id: 8416B1C0014 X-Rspamd-Server: rspam02 X-HE-Tag: 1728636722-977282 X-HE-Meta: U2FsdGVkX1+C6afOW0GCkM3jMEvitZKTv61gvDSHJ9dVLLpxC20nI9yFC58hUfiRr2iJVDLiHbxa8eL+8zSDw8uW6GEWKdBfe/xbCTbu4AiC3nwkDqm0ftqrP/Km1qBLQAXQIrGNjngEg3bo3DAImZzTA+D17IfcbKeIE6i3CoH4r5JQHOKJ8n1taf/Bu6lQxm/8uPoHTEh1rJdeFU5dyr1hT9saoLFjQB0ca/Is4SyuGVN3gJJqaRZBsAWToxxzJ0HmNfHQS8JpaPPyCRzY3XqPWdfSh1J53gt+qjlajkr8+ruftZZkjnXjXjDYEEF+mqzqG7kXc5yN7czH49zh10xg1nJ37OX3cQAWAC9UAfyQ/1uky2422NJi7DrCDbVH8QGp03l268QVClnC6wJlBmwVle6mxl4WMP3TB+TKapXTDlAczQZICVJT0pselRcwGn5ijnLl3DQmG5jF/1lGTJcVR1Lsrmn98dsWYPM4hXTY4fA/9I7UVnKGStNXyAuuzVTbqF4f4E5P8q3NzN5pFNcYqC+k56z9u3XgEnjHzizrEiN+z4LHRsaHOpuaFPzuCdDa2cMYLP+VistPXOQiR6q/8SWo0vZs/TGzQJ+mmEhaG14XjEj4ewa9rrvMwc5HHV2r02C8iCZODUdnjG4x4revicWpI1l6fYwNqzREcCrrGFTfkC/5cdWmjB0d1SoUyCKAzK1DXXPWu64M6DSpZ86UgR5P8M/SqLm+klfy3+9wG8HBj7MhG7fvdRgQ9nea/Tl4IPJ8VlpDCROcnsSFXFjAdTsuu9WAczwfK/7hkmO/ArglLzrwKYG66c/lPYRiBKeiGQpijTsdhT3FVUNOS3bb5pDaOt3bmhjX0Q6OgX7VrKZGPYHdCwhfMbf7wwv0Q2PDwJqmQueKLt4x+EGnpSU7Qm9I7qMmrUa+rDMp65bILzc+4tZgD11LaEONhd1g7l1sv8h2qfBUf4Vumm2 M5zS9pyy lvl33uWTktBSFoey9V4Z1yTw3CI123gbYAY+c5UCWG1s7kgFsFy3o75fyO4rtk11Ih89otOZ+UoQvfwhRIAsJnKk72pTyKcIw9M0otreB0WPWIkrmh35ho9IdReCnSZx99dc/N3xtwCfkCDddOddrnCgbbwElc0lIOVLsWWMWqavBDRLi02GM3H1ygrVyT5i80R0hexBeoV6b/rHksJ4Xd8RFyau6xBhfMoSY4pCcsDR5GjOFCalQ1QTGhg== 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 11.10.24 03:06, Huang, Ying wrote: >> 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. > > Do you mean that we would process it twice in the loop body, or what > exactly do you mean with "evaluate" ? In the macro definition above, _root is used twice. For example, if "_root" is a time consuming function call, the function will run twice. That's not expected. > And just I understand what we want to achieve: we want to walk the > subtree below "root" and prevent going to root->sibling or > root->parent if "root" is not actually the "real root", correct? > > X > |--------| > A----D E > | > B--C > > > So assume we start walking at A, we want to evaluate A,B,C but not D,E,X. > > Does that sum up what we want to achieve? Yes. -- Best Regards, Huang, Ying