From: Andrew Morton <akpm@linux-foundation.org>
To: Minchan Kim <minchan@kernel.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 14:50:11 -0800 [thread overview]
Message-ID: <20130225145011.68e55812.akpm@linux-foundation.org> (raw)
In-Reply-To: <20130225021308.GA6498@blaptop>
On Mon, 25 Feb 2013 11:13:08 +0900
Minchan Kim <minchan@kernel.org> wrote:
> >
> > >
> > > ...
> > >
> > > @@ -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.
Order-0 pages surely preponderate, so I'd say that "likely" is the way
to go.
I don't recall anyone ever demonstrating that likely/unlikely actually
does anything useful. It would be interesting to have a play around,
see if it actually does good things to the code generation.
I think someone (perhaps in or near Dave Jones?) once had a patch which
added counters to likely/unlikely, so the kernel can accumulate and
then report upon the hit/miss ratio at each site. iirc, an alarmingly
large number of the sites were deoptimisations!
> They are trivial patch so send it now or send it after you release
> first mmotm after finishing merge window?
It's in mainline now.
--
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: Andrew Morton <akpm@linux-foundation.org>
To: Minchan Kim <minchan@kernel.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 14:50:11 -0800 [thread overview]
Message-ID: <20130225145011.68e55812.akpm@linux-foundation.org> (raw)
In-Reply-To: <20130225021308.GA6498@blaptop>
On Mon, 25 Feb 2013 11:13:08 +0900
Minchan Kim <minchan@kernel.org> wrote:
> >
> > >
> > > ...
> > >
> > > @@ -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.
Order-0 pages surely preponderate, so I'd say that "likely" is the way
to go.
I don't recall anyone ever demonstrating that likely/unlikely actually
does anything useful. It would be interesting to have a play around,
see if it actually does good things to the code generation.
I think someone (perhaps in or near Dave Jones?) once had a patch which
added counters to likely/unlikely, so the kernel can accumulate and
then report upon the hit/miss ratio at each site. iirc, an alarmingly
large number of the sites were deoptimisations!
> They are trivial patch so send it now or send it after you release
> first mmotm after finishing merge window?
It's in mainline now.
next prev parent reply other threads:[~2013-02-25 22:50 UTC|newest]
Thread overview: 12+ 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 0:16 ` Minchan Kim
2013-01-15 8:43 ` Michal Nazarewicz
2013-01-15 8:43 ` Michal Nazarewicz
2013-01-15 23:36 ` Andrew Morton
2013-01-15 23:36 ` Andrew Morton
2013-02-25 2:13 ` Minchan Kim
2013-02-25 2:13 ` Minchan Kim
2013-02-25 22:50 ` Andrew Morton [this message]
2013-02-25 22:50 ` Andrew Morton
2013-02-26 0:00 ` Minchan Kim
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=20130225145011.68e55812.akpm@linux-foundation.org \
--to=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 \
--cc=minchan@kernel.org \
/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.