Re: [PATCH v3 1/3] kernel/resource: clean up and optimize iomem_is_exclusive()

[Dan Williams]

On Thu, Sep 2, 2021 at 12:52 AM David Hildenbrand <david@xxxxxxxxxx> wrote:
>
> On 01.09.21 21:43, Williams, Dan J wrote:
> > On Tue, 2021-08-31 at 22:21 +0200, David Hildenbrand wrote:
> >> We end up traversing subtrees of ranges we are not interested in; let's
> >> optimize this case, skipping such subtrees, cleaning up the function a bit.
> >>
> >> Signed-off-by: David Hildenbrand <david@xxxxxxxxxx>
> >> ---
> >>   kernel/resource.c | 25 ++++++++++++++++++++-----
> >>   1 file changed, 20 insertions(+), 5 deletions(-)
> >
> > That diffstat does not come across as "cleanup", and the skip_children
> > flag changing values mid-iteration feels tricky. Is there a win here,
> > the same number of entries still need to be accessed, right?
>
> Right, most of the patch changes falls under "optimize". The cleanup is
> using for_each_resource() and not using r_next(NULL, p, &l). Sure, I
> could have split this up but then I'd just introduce for_each_resource()
> to modify it immediately again.
>
>
> Let's take a look at /proc/iomem on my notebook:
>
> 00000000-00000fff : Reserved
> 00001000-00057fff : System RAM
> 00058000-00058fff : Reserved
> 00059000-0009cfff : System RAM
> 0009d000-000fffff : Reserved
>    000a0000-000bffff : PCI Bus 0000:00
>    000c0000-000c3fff : PCI Bus 0000:00
>    000c4000-000c7fff : PCI Bus 0000:00
>    000c8000-000cbfff : PCI Bus 0000:00
>    000cc000-000cffff : PCI Bus 0000:00
>    000d0000-000d3fff : PCI Bus 0000:00
>    000d4000-000d7fff : PCI Bus 0000:00
>    000d8000-000dbfff : PCI Bus 0000:00
>    000dc000-000dffff : PCI Bus 0000:00
>    000e0000-000e3fff : PCI Bus 0000:00
>    000e4000-000e7fff : PCI Bus 0000:00
>    000e8000-000ebfff : PCI Bus 0000:00
>    000ec000-000effff : PCI Bus 0000:00
>    000f0000-000fffff : PCI Bus 0000:00
>      000f0000-000fffff : System ROM
> 00100000-3fffffff : System RAM
> 40000000-403fffff : Reserved
>    40000000-403fffff : pnp 00:00
> 40400000-80a79fff : System RAM
> ...
>
> Why should we take a look at any children of "0009d000-000fffff :
> Reserved" if we can just skip these 15 items directly because the parent
> range is not of interest?

Oh I misread, it never loads the child entries into cache, so it's a
true skip and not a continue.

You can add:

Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>

...I was going to say it should be named for_each_top_resource(), but
we can cross that bridge when / if something needs an iterator that
includes children.