On Thursday, August 12, 2021, David Hildenbrand <[email protected]> wrote:
> On 11.08.21 22:47, Andy Shevchenko wrote: > >> >> >> On Wednesday, August 11, 2021, David Hildenbrand <[email protected] >> <mailto:[email protected]>> 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 <[email protected] >> <mailto:[email protected]>> >> --- >> 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. > > Yes, it’s like micro optimization. If you want your way I suggest then to add a macro #define for_each_iomem_resource_child() \ for (iomem_resource...) > > Thanks, > > David / dhildenb > > -- With Best Regards, Andy Shevchenko
_______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
