linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] for_each_online_pgdat (take2)  [1/5]  define for_each_online_pgdat
@ 2006-02-25  6:05 KAMEZAWA Hiroyuki
  2006-02-25  6:16 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2006-02-25  6:05 UTC (permalink / raw)
  To: LKML; +Cc: Andrew Morton

This patch replaces for_each_pgdat with for_each_online_pgdat()
and makes use of node_online_map instead of pgdat_link.

Changelog v1 -> v2
 - fixes bug in code for ia64 initialization.
 - restructured patch dependency.
 - introduce for_each_online_pgdat().

This patch is against 2.6.16-rc4-mm2. 
tested on i386/ia64(smp)/ia64(NUMA config..but not on NUMA hardware.)

-- Kame

This patch defines for_each_online_pgdat() as a replacement of for_each_pgdat()

Now, online nodes are managed by node_online_map. But for_each_pgdat() uses
pgdat_link to iterate over all nodes(pgdat). This means management structure
for online pgdat is duplicated.

I think using node_online_map for for_each_pgdat() is simple and sane rather
ather than pgdat_link. New macro is named as for_each_online_pgdat().
Following patch will fix callers of for_each_pgdat().

The bootmem allocater uses for_each_pgdat() before pgdat initialization.
I don't think it's sane.Following patch will fix it.

Signed-Off-By: Yasunori Goto     <y-goto@jp.fujitsu.com>
Signed-Off-By: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>


Index: linux-2.6.16-rc4-mm2/include/linux/mmzone.h
===================================================================
--- linux-2.6.16-rc4-mm2.orig/include/linux/mmzone.h
+++ linux-2.6.16-rc4-mm2/include/linux/mmzone.h
@@ -13,6 +13,7 @@
 #include <linux/numa.h>
 #include <linux/init.h>
 #include <linux/seqlock.h>
+#include <linux/nodemask.h>
 #include <asm/atomic.h>
 
 /* Free memory management - zoned buddy allocator.  */
@@ -349,57 +350,6 @@ unsigned long __init node_memmap_size_by
  */
 #define zone_idx(zone)		((zone) - (zone)->zone_pgdat->node_zones)
 
-/**
- * for_each_pgdat - helper macro to iterate over all nodes
- * @pgdat - pointer to a pg_data_t variable
- *
- * Meant to help with common loops of the form
- * pgdat = pgdat_list;
- * while(pgdat) {
- * 	...
- * 	pgdat = pgdat->pgdat_next;
- * }
- */
-#define for_each_pgdat(pgdat) \
-	for (pgdat = pgdat_list; pgdat; pgdat = pgdat->pgdat_next)
-
-/*
- * next_zone - helper magic for for_each_zone()
- * Thanks to William Lee Irwin III for this piece of ingenuity.
- */
-static inline struct zone *next_zone(struct zone *zone)
-{
-	pg_data_t *pgdat = zone->zone_pgdat;
-
-	if (zone < pgdat->node_zones + MAX_NR_ZONES - 1)
-		zone++;
-	else if (pgdat->pgdat_next) {
-		pgdat = pgdat->pgdat_next;
-		zone = pgdat->node_zones;
-	} else
-		zone = NULL;
-
-	return zone;
-}
-
-/**
- * for_each_zone - helper macro to iterate over all memory zones
- * @zone - pointer to struct zone variable
- *
- * The user only needs to declare the zone variable, for_each_zone
- * fills it in. This basically means for_each_zone() is an
- * easier to read version of this piece of code:
- *
- * for (pgdat = pgdat_list; pgdat; pgdat = pgdat->node_next)
- * 	for (i = 0; i < MAX_NR_ZONES; ++i) {
- * 		struct zone * z = pgdat->node_zones + i;
- * 		...
- * 	}
- * }
- */
-#define for_each_zone(zone) \
-	for (zone = pgdat_list->node_zones; zone; zone = next_zone(zone))
-
 static inline int populated_zone(struct zone *zone)
 {
 	return (!!zone->present_pages);
@@ -471,6 +421,62 @@ extern struct pglist_data contig_page_da
 
 #endif /* !CONFIG_NEED_MULTIPLE_NODES */
 
+static inline struct pglist_data *first_online_pgdat(void)
+{
+	return NODE_DATA(first_online_node);
+}
+
+static inline struct pglist_data *next_online_pgdat(struct pglist_data *pgdat)
+{
+	int nid = next_online_node(pgdat->node_id);
+
+	if (nid == MAX_NUMNODES)
+		return NULL;
+	return NODE_DATA(nid);
+}
+
+
+/**
+ * for_each_pgdat - helper macro to iterate over all nodes
+ * @pgdat - pointer to a pg_data_t variable
+ */
+#define for_each_online_pgdat(pgdat)			\
+	for (pgdat = first_online_pgdat();		\
+	     pgdat;					\
+	     pgdat = next_online_pgdat(pgdat))
+
+/*
+ * next_zone - helper magic for for_each_zone()
+ * Thanks to William Lee Irwin III for this piece of ingenuity.
+ */
+static inline struct zone *next_zone(struct zone *zone)
+{
+	pg_data_t *pgdat = zone->zone_pgdat;
+
+	if (zone < pgdat->node_zones + MAX_NR_ZONES - 1)
+		zone++;
+	else {
+		pgdat = next_online_pgdat(pgdat);
+		if (pgdat)
+			zone = pgdat->node_zones;
+		else
+			zone = NULL;
+	}
+	return zone;
+}
+
+/**
+ * for_each_zone - helper macro to iterate over all memory zones
+ * @zone - pointer to struct zone variable
+ *
+ * The user only needs to declare the zone variable, for_each_zone
+ * fills it in.
+ */
+#define for_each_zone(zone)			        \
+	for (zone = (first_online_pgdat())->node_zones; \
+	     zone;					\
+	     zone = next_zone(zone))
+
 #ifdef CONFIG_SPARSEMEM
 #include <asm/sparsemem.h>
 #endif
Index: linux-2.6.16-rc4-mm2/include/linux/nodemask.h
===================================================================
--- linux-2.6.16-rc4-mm2.orig/include/linux/nodemask.h
+++ linux-2.6.16-rc4-mm2/include/linux/nodemask.h
@@ -350,11 +350,15 @@ extern nodemask_t node_possible_map;
 #define num_possible_nodes()	nodes_weight(node_possible_map)
 #define node_online(node)	node_isset((node), node_online_map)
 #define node_possible(node)	node_isset((node), node_possible_map)
+#define first_online_node	first_node(node_online_map)
+#define next_online_node(nid)	next_node((nid), node_online_map)
 #else
 #define num_online_nodes()	1
 #define num_possible_nodes()	1
 #define node_online(node)	((node) == 0)
 #define node_possible(node)	((node) == 0)
+#define first_online_node	0
+#define next_online_node(nid)	(MAX_NUMNODES)
 #endif
 
 #define any_online_node(mask)			\

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] for_each_online_pgdat (take2)  [1/5]  define for_each_online_pgdat
  2006-02-25  6:05 [PATCH] for_each_online_pgdat (take2) [1/5] define for_each_online_pgdat KAMEZAWA Hiroyuki
@ 2006-02-25  6:16 ` Andrew Morton
  2006-02-25  6:22   ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2006-02-25  6:16 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki; +Cc: linux-kernel

KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
> +static inline struct pglist_data *first_online_pgdat(void)
>  +{
>  +	return NODE_DATA(first_online_node);
>  +}
>  +
>  +static inline struct pglist_data *next_online_pgdat(struct pglist_data *pgdat)
>  +{
>  +	int nid = next_online_node(pgdat->node_id);
>  +
>  +	if (nid == MAX_NUMNODES)
>  +		return NULL;
>  +	return NODE_DATA(nid);
>  +}
>  +
>  +
>  +/**
>  + * for_each_pgdat - helper macro to iterate over all nodes
>  + * @pgdat - pointer to a pg_data_t variable
>  + */
>  +#define for_each_online_pgdat(pgdat)			\
>  +	for (pgdat = first_online_pgdat();		\
>  +	     pgdat;					\
>  +	     pgdat = next_online_pgdat(pgdat))
>  +
>  +/*
>  + * next_zone - helper magic for for_each_zone()
>  + * Thanks to William Lee Irwin III for this piece of ingenuity.
>  + */
>  +static inline struct zone *next_zone(struct zone *zone)
>  +{
>  +	pg_data_t *pgdat = zone->zone_pgdat;
>  +
>  +	if (zone < pgdat->node_zones + MAX_NR_ZONES - 1)
>  +		zone++;
>  +	else {
>  +		pgdat = next_online_pgdat(pgdat);
>  +		if (pgdat)
>  +			zone = pgdat->node_zones;
>  +		else
>  +			zone = NULL;
>  +	}
>  +	return zone;
>  +}

Some of these things must generate a large amount of code - would you have
time to look into uninlining them, see what impact that has on .text size?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] for_each_online_pgdat (take2)  [1/5]  define for_each_online_pgdat
  2006-02-25  6:16 ` Andrew Morton
@ 2006-02-25  6:22   ` KAMEZAWA Hiroyuki
  2006-02-25  7:07     ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2006-02-25  6:22 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

On Fri, 24 Feb 2006 22:16:51 -0800
Andrew Morton <akpm@osdl.org> wrote:

 >  +	}
> >  +	return zone;
> >  +}
> 
> Some of these things must generate a large amount of code - would you have
> time to look into uninlining them, see what impact that has on .text size?

Okay, I'll do soon on ia64.

Thanks,
-- Kame


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] for_each_online_pgdat (take2)  [1/5]  define for_each_online_pgdat
  2006-02-25  6:22   ` KAMEZAWA Hiroyuki
@ 2006-02-25  7:07     ` KAMEZAWA Hiroyuki
  2006-02-25  7:30       ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2006-02-25  7:07 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki; +Cc: akpm, linux-kernel

On Sat, 25 Feb 2006 15:22:18 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Fri, 24 Feb 2006 22:16:51 -0800
> Andrew Morton <akpm@osdl.org> wrote:
> > 
> > Some of these things must generate a large amount of code - would you have
> > time to look into uninlining them, see what impact that has on .text size?
> 
> Okay, I'll do soon on ia64.
> 
I compared inlined and out-of-lined vmlinux on ia64 NUMA config kernel.

	inline		out-of-line
.text   005c0680        005bf6a0

005c0680 - 005bf6a0 = FE0 = 4Kbytes. 

Considering the usage of this loop,  above size looks big ;)

I attaches a patch which makes pgdat_walk funcs out-of-line.
I'll rewrite this if necessary.
(make this patch depends on some config or move the place of funcs...)

--Kame

==
Helper function for for_each_online_pgdat / for_each_zone looks too big to
be inlined. Speed of these helper macro itself is not very important.
(inner loops are tend to do more work than this)

This patch make helper function to be out-of-lined.

Signed-Off-By: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>



Index: linux-2.6.16-rc4-mm2/include/linux/mmzone.h
===================================================================
--- linux-2.6.16-rc4-mm2.orig/include/linux/mmzone.h	2006-02-25 13:58:52.000000000 +0900
+++ linux-2.6.16-rc4-mm2/include/linux/mmzone.h	2006-02-25 15:27:49.000000000 +0900
@@ -418,20 +418,9 @@
 
 #endif /* !CONFIG_NEED_MULTIPLE_NODES */
 
-static inline struct pglist_data *first_online_pgdat(void)
-{
-	return NODE_DATA(first_online_node);
-}
-
-static inline struct pglist_data *next_online_pgdat(struct pglist_data *pgdat)
-{
-	int nid = next_online_node(pgdat->node_id);
-
-	if (nid == MAX_NUMNODES)
-		return NULL;
-	return NODE_DATA(nid);
-}
-
+extern struct pglist_data *first_online_pgdat(void);
+extern struct pglist_data *next_online_pgdat(struct pglist_data *pgdat);
+extern struct zone *next_zone(struct zone *zone);
 
 /**
  * for_each_pgdat - helper macro to iterate over all nodes
@@ -441,27 +430,6 @@
 	for (pgdat = first_online_pgdat();		\
 	     pgdat;					\
 	     pgdat = next_online_pgdat(pgdat))
-
-/*
- * next_zone - helper magic for for_each_zone()
- * Thanks to William Lee Irwin III for this piece of ingenuity.
- */
-static inline struct zone *next_zone(struct zone *zone)
-{
-	pg_data_t *pgdat = zone->zone_pgdat;
-
-	if (zone < pgdat->node_zones + MAX_NR_ZONES - 1)
-		zone++;
-	else {
-		pgdat = next_online_pgdat(pgdat);
-		if (pgdat)
-			zone = pgdat->node_zones;
-		else
-			zone = NULL;
-	}
-	return zone;
-}
-
 /**
  * for_each_zone - helper macro to iterate over all memory zones
  * @zone - pointer to struct zone variable
Index: linux-2.6.16-rc4-mm2/mm/mmzone.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.16-rc4-mm2/mm/mmzone.c	2006-02-25 15:32:31.000000000 +0900
@@ -0,0 +1,50 @@
+/*
+ * linux/mm/mmzone.c
+ *
+ * management codes for pgdats and zones.
+ */
+
+
+#include <linux/config.h>
+#include <linux/stddef.h>
+#include <linux/mmzone.h>
+#include <linux/module.h>
+
+struct pglist_data *first_online_pgdat(void)
+{
+	return NODE_DATA(first_online_node);
+}
+
+EXPORT_SYMBOL(first_online_pgdat);
+
+struct pglist_data *next_online_pgdat(struct pglist_data *pgdat)
+{
+	int nid = next_online_node(pgdat->node_id);
+
+	if (nid == MAX_NUMNODES)
+		return NULL;
+	return NODE_DATA(nid);
+}
+EXPORT_SYMBOL(next_online_pgdat);
+
+
+/*
+ * next_zone - helper magic for for_each_zone()
+ */
+struct zone *next_zone(struct zone *zone)
+{
+	pg_data_t *pgdat = zone->zone_pgdat;
+
+	if (zone < pgdat->node_zones + MAX_NR_ZONES - 1)
+		zone++;
+	else {
+		pgdat = next_online_pgdat(pgdat);
+		if (pgdat)
+			zone = pgdat->node_zones;
+		else
+			zone = NULL;
+	}
+	return zone;
+}
+EXPORT_SYMBOL(next_zone);
+



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] for_each_online_pgdat (take2)  [1/5]  define for_each_online_pgdat
  2006-02-25  7:07     ` KAMEZAWA Hiroyuki
@ 2006-02-25  7:30       ` Andrew Morton
  2006-02-25  7:38         ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2006-02-25  7:30 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki; +Cc: kamezawa.hiroyu, linux-kernel

KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
> On Sat, 25 Feb 2006 15:22:18 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
> > On Fri, 24 Feb 2006 22:16:51 -0800
> > Andrew Morton <akpm@osdl.org> wrote:
> > > 
> > > Some of these things must generate a large amount of code - would you have
> > > time to look into uninlining them, see what impact that has on .text size?
> > 
> > Okay, I'll do soon on ia64.
> > 
> I compared inlined and out-of-lined vmlinux on ia64 NUMA config kernel.
> 
> 	inline		out-of-line
> .text   005c0680        005bf6a0
> 
> 005c0680 - 005bf6a0 = FE0 = 4Kbytes. 
> 
> Considering the usage of this loop,  above size looks big ;)

Yes, we inline too many things.  Still.

> I attaches a patch which makes pgdat_walk funcs out-of-line.

Looks fine to me.

> I'll rewrite this if necessary.
> (make this patch depends on some config or move the place of funcs...)

We wouldn't want a config option for it.

And the new mmzone.c probably makes sense too - I expect there are a few
related things (page_alloc.c) which could be moved there.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] for_each_online_pgdat (take2)  [1/5]  define for_each_online_pgdat
  2006-02-25  7:30       ` Andrew Morton
@ 2006-02-25  7:38         ` KAMEZAWA Hiroyuki
  0 siblings, 0 replies; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2006-02-25  7:38 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

On Fri, 24 Feb 2006 23:30:30 -0800
Andrew Morton <akpm@osdl.org> wrote:

> > I'll rewrite this if necessary.
> > (make this patch depends on some config or move the place of funcs...)
> 
> We wouldn't want a config option for it.
> 
> And the new mmzone.c probably makes sense too - I expect there are a few
> related things (page_alloc.c) which could be moved there.

Yes, I'd like to move some of initialization funcs and counting pages funcs to mmzone.c.
Maybe I'll do so for patches related to node-hotplug.

--Kame



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2006-02-25  7:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-02-25  6:05 [PATCH] for_each_online_pgdat (take2) [1/5] define for_each_online_pgdat KAMEZAWA Hiroyuki
2006-02-25  6:16 ` Andrew Morton
2006-02-25  6:22   ` KAMEZAWA Hiroyuki
2006-02-25  7:07     ` KAMEZAWA Hiroyuki
2006-02-25  7:30       ` Andrew Morton
2006-02-25  7:38         ` KAMEZAWA Hiroyuki

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).