LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 1/3] kernel/resource: clean up and optimize iomem_is_exclusive()
Date: Thu, 2 Sep 2021 09:52:16 +0200	[thread overview]
Message-ID: <22be7bd0-3f67-0e9c-87d4-1ec2c184cc6a@redhat.com> (raw)
In-Reply-To: <16141669368a89f8304b1777094e982942d51bba.camel@intel.com>

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@redhat.com>
>> ---
>>   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?


It gets even worse for some PCI buses:

8f800000-f7ffffff : PCI Bus 0000:00
   8f800000-8f9fffff : PCI Bus 0000:01
   8fa00000-8fbfffff : PCI Bus 0000:01
   90000000-b1ffffff : PCI Bus 0000:04
     90000000-b1ffffff : PCI Bus 0000:05
       90000000-b1ffffff : PCI Bus 0000:07
         90000000-b1ffffff : PCI Bus 0000:08
           90000000-b1ffffff : PCI Bus 0000:0d
   c0000000-cfffffff : 0000:00:02.0
   d0000000-e60fffff : PCI Bus 0000:04
     d0000000-e60fffff : PCI Bus 0000:05
       d0000000-e5efffff : PCI Bus 0000:07
         d0000000-e5efffff : PCI Bus 0000:08
           d0000000-d00fffff : PCI Bus 0000:09
           d0000000-d000ffff : 0000:09:00.0
           d0000000-d000ffff : xhci-hcd
           d0010000-d0010fff : 0000:09:00.0
           d0011000-d0011fff : 0000:09:00.0
           d0100000-d01fffff : PCI Bus 0000:0b
           d0100000-d010ffff : 0000:0b:00.0
           d0100000-d010ffff : xhci-hcd
           d0110000-d0110fff : 0000:0b:00.0
           d0111000-d0111fff : 0000:0b:00.0
           d0200000-e5efffff : PCI Bus 0000:0d
       e5f00000-e5ffffff : PCI Bus 0000:3c
         e5f00000-e5f0ffff : 0000:3c:00.0
           e5f00000-e5f0ffff : xhci-hcd
       e6000000-e60fffff : PCI Bus 0000:06
         e6000000-e603ffff : 0000:06:00.0
           e6000000-e603ffff : thunderbolt
         e6040000-e6040fff : 0000:06:00.0
   e7000000-e7ffffff : 0000:00:02.0
   e8000000-e80fffff : PCI Bus 0000:3e
     e8000000-e8003fff : 0000:3e:00.0
       e8000000-e8003fff : nvme
   e8100000-e81fffff : PCI Bus 0000:3d
     e8100000-e8101fff : 0000:3d:00.0
       e8100000-e8101fff : iwlwifi
   e8200000-e821ffff : 0000:00:1f.6
     e8200000-e821ffff : e1000e
   e8220000-e822ffff : 0000:00:14.0
     e8220000-e822ffff : xhci-hcd
       e8228070-e822846f : intel_xhci_usb_sw
   e8230000-e823ffff : 0000:00:1f.3
     e8230000-e823ffff : ICH HD audio
   e8240000-e8247fff : 0000:00:04.0
     e8240000-e8247fff : proc_thermal
   e8248000-e824bfff : 0000:00:1f.3
     e8248000-e824bfff : ICH HD audio
   e824c000-e824ffff : 0000:00:1f.2
   e8250000-e8250fff : 0000:00:08.0
   e8251000-e8251fff : 0000:00:14.2
     e8251000-e8251fff : Intel PCH thermal driver
   e8252000-e8252fff : 0000:00:15.0
     e8252000-e82521ff : lpss_dev
       e8252000-e82521ff : i2c_designware.0 lpss_dev
     e8252200-e82522ff : lpss_priv
     e8252800-e8252fff : idma64.0
       e8252800-e8252fff : idma64.0 idma64.0
   e8253000-e8253fff : 0000:00:16.0
     e8253000-e8253fff : mei_me
   e8254000-e82540ff : 0000:00:1f.4
   f7fe0000-f7ffffff : pnp 00:08
     f7fe0000-f7ffffff : pnp 00:0a

I didn't count how many entries these are, but it's certainly more 
entries in that subtree than I have directly under the root, meaning in 
my setup we end up looking at at least 50% less entries (actually, much 
more).

> 
> BTW, I had to pull this from lore to reply to it, looks like the
> intended Cc's were missing?

Yes, I messed up this time, sorry -- I forgot "--cover-cc" w ... I will 
resend the patches so everybody has them without going trough extra trouble.


-- 
Thanks,

David / dhildenb


  reply	other threads:[~2021-09-02  7:52 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-31 20:21 [PATCH v3 0/3] virtio-mem: disallow mapping virtio-mem memory via /dev/mem David Hildenbrand
2021-08-31 20:21 ` [PATCH v3 1/3] kernel/resource: clean up and optimize iomem_is_exclusive() David Hildenbrand
2021-09-01 19:43   ` Williams, Dan J
2021-09-02  7:52     ` David Hildenbrand [this message]
2021-09-02 15:54       ` Dan Williams
2021-08-31 20:21 ` [PATCH v3 2/3] kernel/resource: disallow access to exclusive system RAM regions David Hildenbrand
2021-09-01 19:37   ` Williams, Dan J
2021-08-31 20:21 ` [PATCH v3 3/3] virtio-mem: disallow mapping virtio-mem memory via /dev/mem David Hildenbrand
2021-09-01 19:39   ` Williams, Dan J

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22be7bd0-3f67-0e9c-87d4-1ec2c184cc6a@redhat.com \
    --to=david@redhat.com \
    --cc=dan.j.williams@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [PATCH v3 1/3] kernel/resource: clean up and optimize iomem_is_exclusive()' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).