From: Christoph Lameter <cl@linux.com>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Pekka Enberg <penberg@kernel.org>, Michal Hocko <mhocko@suse.cz>,
Andrew Morton <akpm@linux-foundation.org>,
Hugh Dickins <hughd@google.com>,
linux-mm@kvack.org, LKML <linux-kernel@vger.kernel.org>,
linux-parisc@vger.kernel.org,
David Rientjes <rientjes@google.com>
Subject: Re: [PATCH v3] mm: make expand_downwards symmetrical to expand_upwards
Date: Tue, 19 Apr 2011 15:56:46 -0500 (CDT) [thread overview]
Message-ID: <alpine.DEB.2.00.1104191530040.23077@router.home> (raw)
In-Reply-To: <1303242580.11237.10.camel@mulgrave.site>
On Tue, 19 Apr 2011, James Bottomley wrote:
> > > I told you ... I forced an allocation into the first discontiguous
> > > region. That will return 1 for page_to_nid().
> >
> > How? The kernel has no concept of a node 1 without CONFIG_NUMA and so you
> > cannot tell the page allocator to allocate from node 1.
>
> Yes, it does, as I explained in the email.
Looked through it and canot find it. How would that be possible to do
with core kernel calls since the page allocator calls do not allow you to
specify a node under !NUMA.
> Don't be silly: alpha, ia64, m32r, m68k, mips, parisc, tile and even x86
> all use the discontigmem memory model in some configurations.
I guess DISCONTIGMEM is typically used together with NUMA. Otherwise we
would have run into this before.
> > > Not really. The rest of the kernel uses the proper macros. in
> > > DISCONTIGMEM but !NUMA configs, the numa macros expand correctly.
> > > You've cut across that with all the CONFIG_NUMA checks in slub.
> >
> > What are "the proper macros"? AFAICT page_to_nid() is the proper way to
> > access the node of a page. If page_to_nid() returns 1 then you have a zone
> > that the kernel knows of as being in node 0 having a page on a different
> > node.
>
> Well it depends what you want. If you only want the actual NUMA node,
> then pfn_to_nid() probably isn't what you want, because in a
> DISCONTIGMEM model, there may be multiple nids per actual numa node.
Right yes you got it. The notion of a node is different(!!!!!). What
matters to the core kernel is the notion of a NUMA node. If DISCONTIGMEM
runs with !NUMA then the way that "node" is used in DISCONTIGMEM is
different from the core code and refers only to the memory blocks managed
by DISCONTIGMEM. The node should be irrelevant to the core then.
> > We can likely force page_to_nid to ignore the node information that have
> > been erroneously placed there but this looks like something deeper is
> > wrong here. The node field in struct page is not only used for the Linux
> > support of a NUMA node but also for blocks of memory. Those should be
> > separate things.
>
> Look, it's not wrong, it's by design. The assumption that non-numa
> systems don't use nodes is the wrong one.
Depends on how you define the notion of a node. The way the core kernel
uses the term "node" means that there will be only one node and that is
node 0 if CONFIG_NUMA is off. Thus page_to_nid() must return 0 for !NUMA.
All sort of things in the core code will break in weird ways if you do
allow page_to_nid to return 1 under !NUMA. Just look at the usage of
page_to_nid(). Tried to use huge pages yet? And how will your version
of reality deal with the following checks in the page allocator? F.e.
VM_BUG_ON(page_to_nid(page) != zone_to_nid(zone));
Enabled CONFIG_DEBUG_VM yet?
> > Index: linux-2.6/include/linux/mm.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/mm.h 2011-04-19 13:20:20.092521248 -0500
> > +++ linux-2.6/include/linux/mm.h 2011-04-19 13:21:05.962521196 -0500
> > @@ -665,6 +665,7 @@ static inline int zone_to_nid(struct zon
> > #endif
> > }
> >
> > +#ifdef CONFIG_NUMA
> > #ifdef NODE_NOT_IN_PAGE_FLAGS
> > extern int page_to_nid(struct page *page);
> > #else
> > @@ -673,6 +674,9 @@ static inline int page_to_nid(struct pag
> > return (page->flags >> NODES_PGSHIFT) & NODES_MASK;
> > }
> > #endif
> > +#else
> > +#define page_to_nid(x) 0
> > +#endif
>
> Don't be silly ... that breaks asm-generic/memory_model.h
Well yeah looks like in order to be clean in the !NUMA case we would then
need a page_to_discontig_node_id() there that is different from the
page_to_nid() used for the core.
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2011-04-19 20:56 UTC|newest]
Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-15 13:51 [PATCH] " Michal Hocko
2011-04-18 3:00 ` Hugh Dickins
2011-04-18 10:01 ` [PATCH v2] " Michal Hocko
2011-04-18 20:56 ` Andrew Morton
[not found] ` <20110419091022.GA21689@tiehlicka.suse.cz>
2011-04-19 11:09 ` [PATCH followup] mm: get rid of CONFIG_STACK_GROWSUP || CONFIG_IA64 Michal Hocko
2011-04-20 0:33 ` KOSAKI Motohiro
2011-04-20 6:59 ` Michal Hocko
2011-04-20 7:08 ` KOSAKI Motohiro
2011-04-26 7:59 ` Michal Hocko
2011-04-19 11:10 ` [PATCH v3] mm: make expand_downwards symmetrical to expand_upwards Michal Hocko
2011-04-19 15:46 ` James Bottomley
2011-04-19 16:06 ` [PATCH v3] mm: make expand_downwards symmetrical to John David Anglin
2011-04-19 16:59 ` James Bottomley
2011-04-19 16:07 ` [PATCH v3] mm: make expand_downwards symmetrical to expand_upwards James Bottomley
2011-04-19 17:05 ` Pekka Enberg
2011-04-19 17:11 ` James Bottomley
2011-04-19 17:15 ` Christoph Lameter
2011-04-19 17:48 ` James Bottomley
2011-04-19 18:10 ` Christoph Lameter
2011-04-19 18:20 ` James Bottomley
2011-04-19 18:35 ` Christoph Lameter
2011-04-19 19:49 ` James Bottomley
2011-04-19 20:56 ` Christoph Lameter [this message]
2011-04-19 21:21 ` James Bottomley
2011-04-19 21:39 ` Christoph Lameter
2011-04-19 21:48 ` James Bottomley
2011-04-19 21:58 ` Christoph Lameter
2011-04-20 1:23 ` KOSAKI Motohiro
2011-04-20 2:48 ` James Bottomley
2011-04-20 2:57 ` KOSAKI Motohiro
2011-04-20 13:50 ` Christoph Lameter
2011-04-21 13:32 ` Tejun Heo
2011-04-20 5:53 ` Pekka Enberg
2011-04-20 7:15 ` KOSAKI Motohiro
2011-04-20 7:34 ` Pekka Enberg
2011-04-20 8:40 ` KOSAKI Motohiro
2011-04-20 16:32 ` James Bottomley
2011-04-20 16:50 ` Christoph Lameter
2011-04-20 18:09 ` James Bottomley
2011-04-20 21:18 ` David Rientjes
2011-04-20 22:15 ` James Bottomley
2011-04-20 23:12 ` David Rientjes
2011-04-21 13:16 ` KOSAKI Motohiro
2011-04-21 16:37 ` James Bottomley
2011-04-21 18:33 ` Christoph Lameter
2011-04-21 18:45 ` Dave Hansen
2011-04-22 18:19 ` James Bottomley
2011-04-22 20:24 ` Dave Hansen
2011-04-22 20:35 ` James Bottomley
2011-04-22 21:33 ` James Bottomley
2011-04-23 18:34 ` [PATCH] convert parisc to sparsemem (was Re: [PATCH v3] mm: make expand_downwards symmetrical to expand_upwards) James Bottomley
2011-04-24 16:27 ` John David Anglin
2011-04-26 0:32 ` KOSAKI Motohiro
2011-04-27 16:36 ` Dave Hansen
2011-04-21 20:05 ` [PATCH v3] mm: make expand_downwards symmetrical to expand_upwards James Bottomley
2011-04-21 21:07 ` Christoph Lameter
2011-04-21 21:22 ` James Bottomley
2011-04-21 19:33 ` David Rientjes
2011-04-22 0:34 ` KOSAKI Motohiro
2011-04-21 13:03 ` KOSAKI Motohiro
2011-04-21 19:38 ` David Rientjes
2011-04-21 20:02 ` Christoph Lameter
2011-04-21 21:19 ` David Rientjes
2011-04-21 21:24 ` James Bottomley
2011-04-21 21:34 ` David Rientjes
2011-04-21 21:49 ` James Bottomley
2011-04-21 22:12 ` David Rientjes
2011-04-22 8:02 ` Pekka Enberg
2011-04-22 13:49 ` James Bottomley
2011-04-22 17:00 ` Pekka Enberg
2011-04-22 17:03 ` James Bottomley
2011-04-21 21:41 ` [patch] mm: always set nodes with regular memory in N_NORMAL_MEMORY David Rientjes
2011-04-22 0:36 ` KOSAKI Motohiro
2011-04-20 21:05 ` [PATCH v3] mm: make expand_downwards symmetrical to expand_upwards David Rientjes
2011-04-20 11:20 ` Matthew Wilcox
2011-04-20 11:28 ` Pekka Enberg
2011-04-20 14:15 ` James Bottomley
2011-04-20 14:50 ` Christoph Lameter
2011-04-20 15:02 ` James Bottomley
2011-04-20 15:22 ` Christoph Lameter
2011-04-20 19:25 ` Matthew Wilcox
2011-04-20 21:42 ` David Rientjes
2011-04-21 16:06 ` James Bottomley
2011-04-21 22:19 ` David Rientjes
2011-04-21 22:31 ` James Bottomley
2011-04-20 13:58 ` Christoph Lameter
2011-04-20 21:34 ` David Rientjes
2011-04-20 14:07 ` Christoph Lameter
2011-04-20 2:33 ` James Bottomley
2011-04-19 17:12 ` Christoph Lameter
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=alpine.DEB.2.00.1104191530040.23077@router.home \
--to=cl@linux.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=akpm@linux-foundation.org \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-parisc@vger.kernel.org \
--cc=mhocko@suse.cz \
--cc=penberg@kernel.org \
--cc=rientjes@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox