2011-02-10 11:46:54

by Namhyung Kim

[permalink] [raw]
Subject: [RFC PATCH] mm: handle simple case in free_pcppages_bulk()

Now I'm seeing that there are some cases to free all pages in a
pcp lists. In that case, just frees all pages in the lists instead
of being bothered with round-robin lists traversal.

Signed-off-by: Namhyung Kim <[email protected]>
---
mm/page_alloc.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e8b02771ccea..959c54450ddf 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -596,6 +596,28 @@ static void free_pcppages_bulk(struct zone *zone, int count,
zone->all_unreclaimable = 0;
zone->pages_scanned = 0;

+ /* Simple case: Free all */
+ if (to_free == pcp->count) {
+ LIST_HEAD(freelist);
+
+ for (; migratetype < MIGRATE_PCPTYPES; migratetype++)
+ if (!list_empty(&pcp->lists[migratetype]))
+ list_move(&pcp->lists[migratetype], &freelist);
+
+ while (!list_empty(&freelist)) {
+ struct page *page;
+
+ page = list_first_entry(&freelist, struct page, lru);
+ /* must delete as __free_one_page list manipulates */
+ list_del(&page->lru);
+ /* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */
+ __free_one_page(page, zone, 0, page_private(page));
+ trace_mm_page_pcpu_drain(page, 0, page_private(page));
+ to_free--;
+ }
+ VM_BUG_ON(to_free);
+ }
+
while (to_free) {
struct page *page;
struct list_head *list;
--
1.7.4


2011-02-10 13:10:40

by Minchan Kim

[permalink] [raw]
Subject: Re: [RFC PATCH] mm: handle simple case in free_pcppages_bulk()

Hello Namhyung,

On Thu, Feb 10, 2011 at 8:46 PM, Namhyung Kim <[email protected]> wrote:
> Now I'm seeing that there are some cases to free all pages in a
> pcp lists. In that case, just frees all pages in the lists instead
> of being bothered with round-robin lists traversal.

I though about that but I didn't send the patch.
That's because many cases which calls free_pcppages_bulk(,
pcp->count,..) are slow path so it adds comparison overhead on fast
path while it loses the effectiveness in slow path.

--
Kind regards,
Minchan Kim

2011-02-10 13:19:00

by Namhyung Kim

[permalink] [raw]
Subject: Re: [RFC PATCH] mm: handle simple case in free_pcppages_bulk()

2011-02-10 (목), 22:10 +0900, Minchan Kim:
> Hello Namhyung,
>

Hi Minchan,


> On Thu, Feb 10, 2011 at 8:46 PM, Namhyung Kim <[email protected]> wrote:
> > Now I'm seeing that there are some cases to free all pages in a
> > pcp lists. In that case, just frees all pages in the lists instead
> > of being bothered with round-robin lists traversal.
>
> I though about that but I didn't send the patch.
> That's because many cases which calls free_pcppages_bulk(,
> pcp->count,..) are slow path so it adds comparison overhead on fast
> path while it loses the effectiveness in slow path.
>

Hmm.. How about adding unlikely() then? Doesn't it help much here?


--
Regards,
Namhyung Kim

2011-02-10 13:39:01

by Minchan Kim

[permalink] [raw]
Subject: Re: [RFC PATCH] mm: handle simple case in free_pcppages_bulk()

On Thu, Feb 10, 2011 at 10:18 PM, Namhyung Kim <[email protected]> wrote:
> 2011-02-10 (목), 22:10 +0900, Minchan Kim:
>> Hello Namhyung,
>>
>
> Hi Minchan,
>
>
>> On Thu, Feb 10, 2011 at 8:46 PM, Namhyung Kim <[email protected]> wrote:
>> > Now I'm seeing that there are some cases to free all pages in a
>> > pcp lists. In that case, just frees all pages in the lists instead
>> > of being bothered with round-robin lists traversal.
>>
>> I though about that but I didn't send the patch.
>> That's because many cases which calls free_pcppages_bulk(,
>> pcp->count,..) are slow path so it adds comparison overhead on fast
>> path while it loses the effectiveness in slow path.
>>
>
> Hmm.. How about adding unlikely() then? Doesn't it help much here?

Yes. It would help but I am not sure how much it is.
AFAIR, when Mel submit the patch, he tried to prove the effectiveness
with some experiment and profiler.
I think if you want it really, we might need some number.
I am not sure it's worth.




--
Kind regards,
Minchan Kim

2011-02-10 13:53:20

by Namhyung Kim

[permalink] [raw]
Subject: Re: [RFC PATCH] mm: handle simple case in free_pcppages_bulk()

2011-02-10 (목), 22:38 +0900, Minchan Kim:
> > Hmm.. How about adding unlikely() then? Doesn't it help much here?
>
> Yes. It would help but I am not sure how much it is.
> AFAIR, when Mel submit the patch, he tried to prove the effectiveness
> with some experiment and profiler.
> I think if you want it really, we might need some number.
> I am not sure it's worth.
>

OK. Thanks for the comments. :)

And it would be really great if you (or somebody) told me how could I
make the numbers on my desktop.


--
Regards,
Namhyung Kim

2011-02-10 14:06:11

by Mel Gorman

[permalink] [raw]
Subject: Re: [RFC PATCH] mm: handle simple case in free_pcppages_bulk()

On Thu, Feb 10, 2011 at 10:38:59PM +0900, Minchan Kim wrote:
> On Thu, Feb 10, 2011 at 10:18 PM, Namhyung Kim <[email protected]> wrote:
> > 2011-02-10 (???), 22:10 +0900, Minchan Kim:
> >> Hello Namhyung,
> >>
> >
> > Hi Minchan,
> >
> >
> >> On Thu, Feb 10, 2011 at 8:46 PM, Namhyung Kim <[email protected]> wrote:
> >> > Now I'm seeing that there are some cases to free all pages in a
> >> > pcp lists. In that case, just frees all pages in the lists instead
> >> > of being bothered with round-robin lists traversal.
> >>
> >> I though about that but I didn't send the patch.
> >> That's because many cases which calls free_pcppages_bulk(,
> >> pcp->count,..) are slow path so it adds comparison overhead on fast
> >> path while it loses the effectiveness in slow path.
> >>
> >
> > Hmm.. How about adding unlikely() then? Doesn't it help much here?
>
> Yes. It would help but I am not sure how much it is.
> AFAIR, when Mel submit the patch, he tried to prove the effectiveness
> with some experiment and profiler.

Yep. Principally I *think* used netperf running UDP_STREAM for different
buffer sizes and compared oprofile output but I also ran a battery of
benchmarks to check for any other unexpected regression without profiling.

--
Mel Gorman
SUSE Labs