From: Minchan Kim <minchan@kernel.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
Michal Nazarewicz <mina86@mina86.com>
Subject: Re: [PATCH v2] mm: remove MIGRATE_ISOLATE check in hotpath
Date: Mon, 25 Feb 2013 11:13:08 +0900 [thread overview]
Message-ID: <20130225021308.GA6498@blaptop> (raw)
In-Reply-To: <20130115153625.96265439.akpm@linux-foundation.org>
Hi Andrew,
Sorry for late reply and I totally missed it. :(
On Tue, Jan 15, 2013 at 03:36:25PM -0800, Andrew Morton wrote:
> On Tue, 15 Jan 2013 09:16:46 +0900
> Minchan Kim <minchan@kernel.org> wrote:
>
> > Now mm several functions test MIGRATE_ISOLATE and some of those
> > are hotpath but MIGRATE_ISOLATE is used only if we enable
> > CONFIG_MEMORY_ISOLATION(ie, CMA, memory-hotplug and memory-failure)
> > which are not common config option. So let's not add unnecessary
> > overhead and code when we don't enable CONFIG_MEMORY_ISOLATION.
>
> ugh. Better than nothing, I guess.
>
> There remain call sites which do open-coded
>
> get_pageblock_migratetype(page) != MIGRATE_ISOLATE
>
> (undo_isolate_page_range() is one). Wanna clean these up as well?
Oops, Sure.
>
> >
> > ...
> >
> > @@ -683,7 +683,7 @@ static void free_one_page(struct zone *zone, struct page *page, int order,
> > zone->pages_scanned = 0;
> >
> > __free_one_page(page, zone, order, migratetype);
> > - if (unlikely(migratetype != MIGRATE_ISOLATE))
> > + if (unlikely(!is_migrate_isolate(migratetype)))
> > __mod_zone_freepage_state(zone, 1 << order, migratetype);
> > spin_unlock(&zone->lock);
> > }
>
> The code both before and after this patch is assuming that the
> migratetype in free_one_page is likely to be MIGRATE_ISOLATE. Seems
> wrong. If CONFIG_MEMORY_ISOLATION=n this ends up doing
> if(unlikely(true)) which is harmless-but-amusing.
>From the beginning of [2139cbe627, cma: fix counting of isolated pages],
it was wrong. We can't make sure it's very likely.
If it is called by order-0 page free path, it is but if it is called by
high order page free path, we can't.
So I think it would be better to remove unlikley.
They are trivial patch so send it now or send it after you release
first mmotm after finishing merge window?
Andrew, Which is better?
>
> --
> 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>
--
Kind regards,
Minchan Kim
--
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>
next prev parent reply other threads:[~2013-02-25 2:13 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-15 0:16 [PATCH v2] mm: remove MIGRATE_ISOLATE check in hotpath Minchan Kim
2013-01-15 8:43 ` Michal Nazarewicz
2013-01-15 23:36 ` Andrew Morton
2013-02-25 2:13 ` Minchan Kim [this message]
2013-02-25 22:50 ` Andrew Morton
2013-02-26 0:00 ` Minchan Kim
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=20130225021308.GA6498@blaptop \
--to=minchan@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mina86@mina86.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 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).