On 11.08.21 22:47, Andy Shevchenko wrote:
On Wednesday, August 11, 2021, David Hildenbrand <david@xxxxxxxxxx <mailto:david@xxxxxxxxxx>> wrote:Let's clean it up a bit, removing the unnecessary usage of r_next() by next_resource(), and use next_range_resource() in case we are not interested in a certain subtree. Signed-off-by: David Hildenbrand <david@xxxxxxxxxx <mailto:david@xxxxxxxxxx>> --- kernel/resource.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/kernel/resource.c b/kernel/resource.c index 2938cf520ca3..ea853a075a83 100644 --- a/kernel/resource.c +++ b/kernel/resource.c @@ -1754,9 +1754,8 @@ static int strict_iomem_checks; */ bool iomem_is_exclusive(u64 addr) { - struct resource *p = &iomem_resource; + struct resource *p; bool err = false; - loff_t l; int size = PAGE_SIZE; if (!strict_iomem_checks) @@ -1765,27 +1764,31 @@ bool iomem_is_exclusive(u64 addr) addr = addr & PAGE_MASK; read_lock(&resource_lock); - for (p = p->child; p ; p = r_next(NULL, p, &l)) { + for (p = iomem_resource.child; p ;) {
Hi Andy,
I consider the ordinal part of p initialization is slightly better and done outside of read lock.Something like p= &iomem_res...; read lock for (p = p->child; ...) {
Why should we care about doing that outside of the lock? That smells like a micro-optimization the compiler will most probably overwrite either way as the address of iomem_resource is just constant?
Also, for me it's much more readable and compact if we perform a single initialization instead of two separate ones in this case.
We're using the pattern I use in, find_next_iomem_res() and __region_intersects(), while we use the old pattern in iomem_map_sanity_check(), where we also use the same unnecessary r_next() call.
I might just cleanup iomem_map_sanity_check() in a similar way. -- Thanks, David / dhildenb
