Re: [PATCH -v3 2/3] resource: Make alloc_free_mem_region() works for iomem_resource

From: David Hildenbrand
Date: Mon Sep 09 2024 - 03:04:35 EST


On 06.09.24 05:07, Huang Ying wrote:
During developing a kunit test case for region_intersects(), some fake
resources need to be inserted into iomem_resource. To do that, a
resource hole needs to be found first in iomem_resource.

However, alloc_free_mem_region() cannot work for iomem_resource now.
Because the start address to check cannot be 0 to detect address
wrapping 0 in gfr_continue(), while iomem_resource.start == 0. To
make alloc_free_mem_region() works for iomem_resource, gfr_start() is
changed to avoid to return 0 even if base->start == 0. We don't need
to check 0 as start address.

Signed-off-by: "Huang, Ying" <ying.huang@xxxxxxxxx>
Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
Cc: David Hildenbrand <david@xxxxxxxxxx>
Cc: Davidlohr Bueso <dave@xxxxxxxxxxxx>
Cc: Jonathan Cameron <jonathan.cameron@xxxxxxxxxx>
Cc: Dave Jiang <dave.jiang@xxxxxxxxx>
Cc: Alison Schofield <alison.schofield@xxxxxxxxx>
Cc: Vishal Verma <vishal.l.verma@xxxxxxxxx>
Cc: Ira Weiny <ira.weiny@xxxxxxxxx>
Cc: Alistair Popple <apopple@xxxxxxxxxx>
Cc: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Cc: Baoquan He <bhe@xxxxxxxxxx>
---
kernel/resource.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 235dc77f8add..035ef16c1a66 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1873,7 +1873,7 @@ static resource_size_t gfr_start(struct resource *base, resource_size_t size,
return end - size + 1;
}
- return ALIGN(base->start, align);

You should add a comment here. But I do find what you are doing here quite confusing.

Above you write: "We don't need to check 0 as start address." -- why? To make the code extra confusing? :)

/* Never return address 0, because XXX. */
if (!base->start)
retrn align;
return ALIGN(base->start, align);


And i still haven't understood XXX. For whom exactly is address 0 a problem?

+ return ALIGN(max(base->start, align), align);
}
static bool gfr_continue(struct resource *base, resource_size_t addr,

--
Cheers,

David / dhildenb