From: Huang Ying <ying.huang@intel.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
linux-cxl@vger.kernel.org, Huang Ying <ying.huang@intel.com>,
Dan Williams <dan.j.williams@intel.com>,
David Hildenbrand <david@redhat.com>,
Davidlohr Bueso <dave@stgolabs.net>,
Jonathan Cameron <jonathan.cameron@huawei.com>,
Alistair Popple <apopple@nvidia.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Bjorn Helgaas <bhelgaas@google.com>, Baoquan He <bhe@redhat.com>,
Dave Jiang <dave.jiang@intel.com>,
Alison Schofield <alison.schofield@intel.com>
Subject: [PATCH] resource: Avoid unnecessary resource tree walking in __region_intersects()
Date: Tue, 22 Oct 2024 13:38:35 +0800 [thread overview]
Message-ID: <20241022053835.217703-1-ying.huang@intel.com> (raw)
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 descendant 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 makes
it possible to only traverse the subtree under the specified resource.
Test shows that this avoids unnecessary resource tree walking in
__region_intersects().
For the example resource tree as follows,
X
|
A----D----E
|
B--C
if 'A' is the overlapped but unmatched resource, original kernel
iterates 'B', 'C', 'D', 'E' when it walks the descendant tree. While
the patched kernel iterates only 'B', 'C'.
It appears even better to revise for_each_resource() to traverse the
resource subtree under "_root" only. But that will cause "_root" to
be evaluated twice, which I don't find a good way to eliminate.
Thanks David Hildenbrand for providing a good resource tree example.
Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: Alistair Popple <apopple@nvidia.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Alison Schofield <alison.schofield@intel.com>
---
Changes:
RFC->v1:
- Revised patch description and comments, Thanks David and Andy!
- Link to RFC: https://lore.kernel.org/linux-mm/20241010065558.1347018-1-ying.huang@intel.com/
---
kernel/resource.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/kernel/resource.c b/kernel/resource.c
index b730bd28b422..bd217d57fb09 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -50,15 +50,34 @@ EXPORT_SYMBOL(iomem_resource);
static DEFINE_RWLOCK(resource_lock);
-static struct resource *next_resource(struct resource *p, bool skip_children)
+/*
+ * Return the next node of @p in pre-order tree traversal. If
+ * @skip_children is true, skip the descendant nodes of @p in
+ * traversal. If @p is a descendant of @subtree_root, only traverse
+ * the subtree under @subtree_root.
+ */
+static struct resource *__next_resource(struct resource *p, bool skip_children,
+ struct resource *subtree_root)
{
if (!skip_children && p->child)
return p->child;
- while (!p->sibling && p->parent)
+ while (!p->sibling && p->parent) {
p = p->parent;
+ if (p == subtree_root)
+ return NULL;
+ }
return p->sibling;
}
+static struct resource *next_resource(struct resource *p, bool skip_children)
+{
+ return __next_resource(p, skip_children, NULL);
+}
+
+/*
+ * Traverse the whole resource tree with @_root as root in pre-order.
+ * NOTE: @_root should be the topmost node, that is, @_root->parent == NULL.
+ */
#define for_each_resource(_root, _p, _skip_children) \
for ((_p) = (_root)->child; (_p); (_p) = next_resource(_p, _skip_children))
@@ -572,7 +591,8 @@ static int __region_intersects(struct resource *parent, resource_size_t start,
covered = false;
ostart = max(res.start, p->start);
oend = min(res.end, p->end);
- for_each_resource(p, dp, false) {
+ /* Traverse the subtree under 'p'. */
+ for (dp = p->child; dp; dp = __next_resource(dp, false, p)) {
if (!resource_overlaps(dp, &res))
continue;
is_type = (dp->flags & flags) == flags &&
--
2.39.2
next reply other threads:[~2024-10-22 5:38 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-22 5:38 Huang Ying [this message]
2024-10-22 8:19 ` Dan Williams
2024-10-22 8:51 ` Huang, Ying
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241022053835.217703-1-ying.huang@intel.com \
--to=ying.huang@intel.com \
--cc=akpm@linux-foundation.org \
--cc=alison.schofield@intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=apopple@nvidia.com \
--cc=bhe@redhat.com \
--cc=bhelgaas@google.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=david@redhat.com \
--cc=jonathan.cameron@huawei.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox