On 11.08.21 22:47, Andy Shevchenko wrote:

On Wednesday, August 11, 2021, David Hildenbrand <david@redhat.com 
<mailto:david@redhat.com>> 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@redhat.com
    <mailto:david@redhat.com>>
    ---
      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