From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Michel Lespinasse <walken@google.com>
Cc: Rik van Riel <riel@redhat.com>,
"James E.J. Bottomley" <jejb@parisc-linux.org>,
Matt Turner <mattst88@gmail.com>,
David Howells <dhowells@redhat.com>,
Tony Luck <tony.luck@intel.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
linuxppc-dev@lists.ozlabs.org, linux-parisc@vger.kernel.org,
linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org
Subject: Re: [PATCH 7/8] mm: use vm_unmapped_area() on powerpc architecture
Date: Wed, 09 Jan 2013 14:32:56 +1100 [thread overview]
Message-ID: <1357702376.4838.32.camel@pasglop> (raw)
In-Reply-To: <CANN689EJV_7Q7J4j1ttDxZuqbwD53PAuCHb5DhiE-AVbmNSR7Q@mail.gmail.com>
On Tue, 2013-01-08 at 18:38 -0800, Michel Lespinasse wrote:
>
> Well no fair, the previous patch (for powerpc as well) has 22
> insertions and 93 deletions :)
>
> The benefit is that the new code has lower algorithmic complexity, it
> replaces a per-vma loop with O(N) complexity with an outer loop that
> finds contiguous slice blocks and passes them to vm_unmapped_area()
> which is only O(log N) complexity. So the new code will be faster for
> workloads which use lots of vmas.
>
> That said, I do agree that the code that looks for contiguous
> available slices looks kinda ugly - just not sure how to make it look
> nicer though.
Ok. I think at least you can move that construct:
+ if (addr < SLICE_LOW_TOP) {
+ slice = GET_LOW_SLICE_INDEX(addr);
+ addr = (slice + 1) << SLICE_LOW_SHIFT;
+ if (!(available.low_slices & (1u << slice)))
+ continue;
+ } else {
+ slice = GET_HIGH_SLICE_INDEX(addr);
+ addr = (slice + 1) << SLICE_HIGH_SHIFT;
+ if (!(available.high_slices & (1u << slice)))
+ continue;
+ }
Into some kind of helper. It will probably compile to the same thing but
at least it's more readable and it will avoid a fuckup in the future if
somebody changes the algorithm and forgets to update one of the
copies :-)
Cheers,
Ben.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Michel Lespinasse <walken@google.com>
Cc: Rik van Riel <riel@redhat.com>,
"James E.J. Bottomley" <jejb@parisc-linux.org>,
Matt Turner <mattst88@gmail.com>,
David Howells <dhowells@redhat.com>,
Tony Luck <tony.luck@intel.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
linuxppc-dev@lists.ozlabs.org, linux-parisc@vger.kernel.org,
linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org
Subject: Re: [PATCH 7/8] mm: use vm_unmapped_area() on powerpc architecture
Date: Wed, 09 Jan 2013 03:32:56 +0000 [thread overview]
Message-ID: <1357702376.4838.32.camel@pasglop> (raw)
In-Reply-To: <CANN689EJV_7Q7J4j1ttDxZuqbwD53PAuCHb5DhiE-AVbmNSR7Q@mail.gmail.com>
On Tue, 2013-01-08 at 18:38 -0800, Michel Lespinasse wrote:
>
> Well no fair, the previous patch (for powerpc as well) has 22
> insertions and 93 deletions :)
>
> The benefit is that the new code has lower algorithmic complexity, it
> replaces a per-vma loop with O(N) complexity with an outer loop that
> finds contiguous slice blocks and passes them to vm_unmapped_area()
> which is only O(log N) complexity. So the new code will be faster for
> workloads which use lots of vmas.
>
> That said, I do agree that the code that looks for contiguous
> available slices looks kinda ugly - just not sure how to make it look
> nicer though.
Ok. I think at least you can move that construct:
+ if (addr < SLICE_LOW_TOP) {
+ slice = GET_LOW_SLICE_INDEX(addr);
+ addr = (slice + 1) << SLICE_LOW_SHIFT;
+ if (!(available.low_slices & (1u << slice)))
+ continue;
+ } else {
+ slice = GET_HIGH_SLICE_INDEX(addr);
+ addr = (slice + 1) << SLICE_HIGH_SHIFT;
+ if (!(available.high_slices & (1u << slice)))
+ continue;
+ }
Into some kind of helper. It will probably compile to the same thing but
at least it's more readable and it will avoid a fuckup in the future if
somebody changes the algorithm and forgets to update one of the
copies :-)
Cheers,
Ben.
WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Michel Lespinasse <walken@google.com>
Cc: Rik van Riel <riel@redhat.com>, Tony Luck <tony.luck@intel.com>,
linux-ia64@vger.kernel.org, linux-parisc@vger.kernel.org,
"James E.J. Bottomley" <jejb@parisc-linux.org>,
linux-kernel@vger.kernel.org, David Howells <dhowells@redhat.com>,
linux-mm@kvack.org, linux-alpha@vger.kernel.org,
Matt Turner <mattst88@gmail.com>,
linuxppc-dev@lists.ozlabs.org,
Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 7/8] mm: use vm_unmapped_area() on powerpc architecture
Date: Wed, 09 Jan 2013 14:32:56 +1100 [thread overview]
Message-ID: <1357702376.4838.32.camel@pasglop> (raw)
In-Reply-To: <CANN689EJV_7Q7J4j1ttDxZuqbwD53PAuCHb5DhiE-AVbmNSR7Q@mail.gmail.com>
On Tue, 2013-01-08 at 18:38 -0800, Michel Lespinasse wrote:
>
> Well no fair, the previous patch (for powerpc as well) has 22
> insertions and 93 deletions :)
>
> The benefit is that the new code has lower algorithmic complexity, it
> replaces a per-vma loop with O(N) complexity with an outer loop that
> finds contiguous slice blocks and passes them to vm_unmapped_area()
> which is only O(log N) complexity. So the new code will be faster for
> workloads which use lots of vmas.
>
> That said, I do agree that the code that looks for contiguous
> available slices looks kinda ugly - just not sure how to make it look
> nicer though.
Ok. I think at least you can move that construct:
+ if (addr < SLICE_LOW_TOP) {
+ slice = GET_LOW_SLICE_INDEX(addr);
+ addr = (slice + 1) << SLICE_LOW_SHIFT;
+ if (!(available.low_slices & (1u << slice)))
+ continue;
+ } else {
+ slice = GET_HIGH_SLICE_INDEX(addr);
+ addr = (slice + 1) << SLICE_HIGH_SHIFT;
+ if (!(available.high_slices & (1u << slice)))
+ continue;
+ }
Into some kind of helper. It will probably compile to the same thing but
at least it's more readable and it will avoid a fuckup in the future if
somebody changes the algorithm and forgets to update one of the
copies :-)
Cheers,
Ben.
WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Michel Lespinasse <walken@google.com>
Cc: Rik van Riel <riel@redhat.com>,
"James E.J. Bottomley" <jejb@parisc-linux.org>,
Matt Turner <mattst88@gmail.com>,
David Howells <dhowells@redhat.com>,
Tony Luck <tony.luck@intel.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
linuxppc-dev@lists.ozlabs.org, linux-parisc@vger.kernel.org,
linux-alpha@vger.kernel.org, linux-ia64@vger.kernel.org
Subject: Re: [PATCH 7/8] mm: use vm_unmapped_area() on powerpc architecture
Date: Wed, 09 Jan 2013 14:32:56 +1100 [thread overview]
Message-ID: <1357702376.4838.32.camel@pasglop> (raw)
In-Reply-To: <CANN689EJV_7Q7J4j1ttDxZuqbwD53PAuCHb5DhiE-AVbmNSR7Q@mail.gmail.com>
On Tue, 2013-01-08 at 18:38 -0800, Michel Lespinasse wrote:
>
> Well no fair, the previous patch (for powerpc as well) has 22
> insertions and 93 deletions :)
>
> The benefit is that the new code has lower algorithmic complexity, it
> replaces a per-vma loop with O(N) complexity with an outer loop that
> finds contiguous slice blocks and passes them to vm_unmapped_area()
> which is only O(log N) complexity. So the new code will be faster for
> workloads which use lots of vmas.
>
> That said, I do agree that the code that looks for contiguous
> available slices looks kinda ugly - just not sure how to make it look
> nicer though.
Ok. I think at least you can move that construct:
+ if (addr < SLICE_LOW_TOP) {
+ slice = GET_LOW_SLICE_INDEX(addr);
+ addr = (slice + 1) << SLICE_LOW_SHIFT;
+ if (!(available.low_slices & (1u << slice)))
+ continue;
+ } else {
+ slice = GET_HIGH_SLICE_INDEX(addr);
+ addr = (slice + 1) << SLICE_HIGH_SHIFT;
+ if (!(available.high_slices & (1u << slice)))
+ continue;
+ }
Into some kind of helper. It will probably compile to the same thing but
at least it's more readable and it will avoid a fuckup in the future if
somebody changes the algorithm and forgets to update one of the
copies :-)
Cheers,
Ben.
next prev parent reply other threads:[~2013-01-09 3:32 UTC|newest]
Thread overview: 100+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-09 1:28 [PATCH 0/8] vm_unmapped_area: finish the mission Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` [PATCH 1/8] mm: use vm_unmapped_area() on parisc architecture Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 16:56 ` Rik van Riel
2013-01-09 16:56 ` Rik van Riel
2013-01-09 16:56 ` Rik van Riel
2013-01-09 16:56 ` Rik van Riel
2013-01-09 1:28 ` [PATCH 2/8] mm: use vm_unmapped_area() on alpha architecture Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 17:01 ` Rik van Riel
2013-01-09 17:01 ` Rik van Riel
2013-01-09 17:01 ` Rik van Riel
2013-01-09 17:01 ` Rik van Riel
2013-01-25 3:49 ` Michael Cree
2013-01-25 3:49 ` Michael Cree
2013-01-25 3:49 ` Michael Cree
2013-01-25 3:49 ` Michael Cree
2013-01-09 1:28 ` [PATCH 3/8] mm: use vm_unmapped_area() on frv architecture Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 18:25 ` Rik van Riel
2013-01-09 18:25 ` Rik van Riel
2013-01-09 18:25 ` Rik van Riel
2013-01-09 18:25 ` Rik van Riel
2013-01-09 1:28 ` [PATCH 4/8] mm: use vm_unmapped_area() on ia64 architecture Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 18:29 ` Rik van Riel
2013-01-09 18:29 ` Rik van Riel
2013-01-09 18:29 ` Rik van Riel
2013-01-09 18:29 ` Rik van Riel
2013-01-09 1:28 ` [PATCH 5/8] mm: use vm_unmapped_area() in hugetlbfs " Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 18:32 ` Rik van Riel
2013-01-09 18:32 ` Rik van Riel
2013-01-09 18:32 ` Rik van Riel
2013-01-09 18:32 ` Rik van Riel
2013-01-09 1:28 ` [PATCH 6/8] mm: remove free_area_cache use in powerpc architecture Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 20:57 ` Rik van Riel
2013-01-09 20:57 ` Rik van Riel
2013-01-09 20:57 ` Rik van Riel
2013-01-09 20:57 ` Rik van Riel
2013-01-09 1:28 ` [PATCH 7/8] mm: use vm_unmapped_area() on " Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 2:15 ` Benjamin Herrenschmidt
2013-01-09 2:15 ` Benjamin Herrenschmidt
2013-01-09 2:15 ` Benjamin Herrenschmidt
2013-01-09 2:15 ` Benjamin Herrenschmidt
2013-01-09 2:38 ` Michel Lespinasse
2013-01-09 2:38 ` Michel Lespinasse
2013-01-09 2:38 ` Michel Lespinasse
2013-01-09 2:38 ` Michel Lespinasse
2013-01-09 3:32 ` Benjamin Herrenschmidt [this message]
2013-01-09 3:32 ` Benjamin Herrenschmidt
2013-01-09 3:32 ` Benjamin Herrenschmidt
2013-01-09 3:32 ` Benjamin Herrenschmidt
2013-01-09 11:23 ` Michel Lespinasse
2013-01-09 11:23 ` Michel Lespinasse
2013-01-09 11:23 ` Michel Lespinasse
2013-01-09 11:23 ` Michel Lespinasse
2013-01-09 21:41 ` Rik van Riel
2013-01-09 21:41 ` Rik van Riel
2013-01-09 21:41 ` Rik van Riel
2013-01-09 21:41 ` Rik van Riel
2013-01-09 21:24 ` Rik van Riel
2013-01-09 21:24 ` Rik van Riel
2013-01-09 21:24 ` Rik van Riel
2013-01-09 21:24 ` Rik van Riel
2013-01-09 1:28 ` [PATCH 8/8] mm: remove free_area_cache Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 1:28 ` Michel Lespinasse
2013-01-09 21:25 ` Rik van Riel
2013-01-09 21:25 ` Rik van Riel
2013-01-09 21:25 ` Rik van Riel
2013-01-09 21:25 ` Rik van Riel
2013-01-09 1:32 ` [PATCH 0/8] vm_unmapped_area: finish the mission Michel Lespinasse
2013-01-09 1:32 ` Michel Lespinasse
2013-01-09 1:32 ` Michel Lespinasse
2013-01-09 1:32 ` Michel Lespinasse
-- strict thread matches above, loose matches on Subject: below --
2013-01-24 1:29 [PATCH 0/8] convert remaining archs to use vm_unmapped_area() Michel Lespinasse
2013-01-24 1:29 ` [PATCH 7/8] mm: use vm_unmapped_area() on powerpc architecture Michel Lespinasse
2013-01-24 1:29 ` Michel Lespinasse
2013-01-24 1:29 ` Michel Lespinasse
2013-01-24 1:29 ` Michel Lespinasse
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=1357702376.4838.32.camel@pasglop \
--to=benh@kernel.crashing.org \
--cc=akpm@linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=jejb@parisc-linux.org \
--cc=linux-alpha@vger.kernel.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-parisc@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mattst88@gmail.com \
--cc=riel@redhat.com \
--cc=tony.luck@intel.com \
--cc=walken@google.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.