From: Michal Nazarewicz <mina86@mina86.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: compare MIGRATE_ISOLATE selectively
Date: Fri, 21 Dec 2012 13:46:23 +0100 [thread overview]
Message-ID: <xa1tr4mjpo80.fsf@mina86.com> (raw)
In-Reply-To: <20121221010902.GD2686@blaptop>
[-- Attachment #1: Type: text/plain, Size: 2110 bytes --]
> On Thu, Dec 20, 2012 at 04:49:44PM +0100, Michal Nazarewicz wrote:
>> Perhaps “is_migrate_isolate” to match already existing “is_migrate_cma”?
On Fri, Dec 21 2012, Minchan Kim wrote:
> Good poking. In fact, while I made this patch, I was very tempted by renaming
> is_migrate_cma to cma_pageblock.
>
> is_migrate_cma(mt)
>
> I don't know who start to use "mt" instead of "migratetype" but anyway, it's
> not a good idea.
>
> is_migrate_cma(migratetype)
>
> It's very clear for me because migratetype is per pageblock, we can know the
> function works per pageblock unit.
>
>> Especially as the “mt_isolated_pageblock” sound confusing to me, it
>> implies that it works on pageblocks which it does not.
>
> -ENOPARSE.
>
> migratetype works on pageblock.
migratetype is a number, which can be assigned to a pageblock. In some
transitional cases, the migratetype associated with a page can differ
from the migratetype associated with the pageblock the page is in. As
such, I think it's confusing to add “pageblock” to the name of the
function which does not read migratetype from pageblock but rather
operates on the number it is provided.
> I admit mt is really dirty but I used page_alloc.c already has lots of
> mt, SIGH.
I don't really have an issue with “mt” myself, especially since the few
times “mt” is used in page_alloc.c it is a local variable which I don't
think needs a long descriptive name since context is all there.
> How about this?
>
> 1. Let's change all "mt" with "migratetype" again.
> 2. use is_migrate_isolate and is_migrate_cma for "migratetype".
> 3. use is_migrate_isolate_page instead of page_isolated_pageblock for
> "page".
Like I've said. Personally I don't really think 1 is needed, but 2 and
3 look good to me.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--
[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]
next prev parent reply other threads:[~2012-12-21 12:46 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-20 5:25 [PATCH] mm: compare MIGRATE_ISOLATE selectively Minchan Kim
2012-12-20 5:25 ` Minchan Kim
2012-12-20 15:49 ` Michal Nazarewicz
2012-12-20 15:49 ` Michal Nazarewicz
2012-12-21 1:09 ` Minchan Kim
2012-12-21 1:09 ` Minchan Kim
2012-12-21 12:46 ` Michal Nazarewicz [this message]
2012-12-23 23:26 ` Minchan Kim
2012-12-23 23:26 ` 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=xa1tr4mjpo80.fsf@mina86.com \
--to=mina86@mina86.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--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.