Re: [PATCH 20/35] Use a pre-calculated value for num_online_nodes()

From: Mel Gorman
Date: Thu Mar 19 2009 - 18:07:41 EST


On Thu, Mar 19, 2009 at 04:43:55PM -0400, Christoph Lameter wrote:
>
> It seems that its beneficial to have a less expensive way to check for the
> number of currently active and possible nodes in a NUMA system. This will
> simplify further kernel optimizations for the cases in which only a single
> node is online or possible.
>
> Signed-off-by: Christoph Lameter <cl@xxxxxxxxxxxxxxxxxxxx>
>
> Index: linux-2.6/include/linux/nodemask.h
> ===================================================================
> --- linux-2.6.orig/include/linux/nodemask.h 2009-03-19 15:04:28.000000000 -0500
> +++ linux-2.6/include/linux/nodemask.h 2009-03-19 15:33:18.000000000 -0500
> @@ -408,6 +408,20 @@
> #define next_online_node(nid) next_node((nid), node_states[N_ONLINE])
>
> extern int nr_node_ids;
> +extern int nr_online_nodes;
> +extern int nr_possible_nodes;

Have you tested the nr_possible_nodes aspects of this patch? I can see
where it gets initialised but nothing that updates it. It would appear that
nr_possible_nodes() and num_possible_nodes() can return different values.

> +
> +static inline void node_set_online(int nid)
> +{
> + node_set_state(nid, N_ONLINE);
> + nr_online_nodes = num_node_state(N_ONLINE);
> +}
> +
> +static inline void node_set_offline(int nid)
> +{
> + node_clear_state(nid, N_ONLINE);
> + nr_online_nodes = num_node_state(N_ONLINE);
> +}
> #else
>
> static inline int node_state(int node, enum node_states state)
> @@ -434,7 +448,8 @@
> #define first_online_node 0
> #define next_online_node(nid) (MAX_NUMNODES)
> #define nr_node_ids 1
> -
> +#define nr_online_nodes 1
> +#define nr_possible_nodes 1
> #endif
>
> #define node_online_map node_states[N_ONLINE]
> @@ -454,8 +469,7 @@
> #define node_online(node) node_state((node), N_ONLINE)
> #define node_possible(node) node_state((node), N_POSSIBLE)
>
> -#define node_set_online(node) node_set_state((node), N_ONLINE)
> -#define node_set_offline(node) node_clear_state((node), N_ONLINE)
> +
>
> #define for_each_node(node) for_each_node_state(node, N_POSSIBLE)
> #define for_each_online_node(node) for_each_node_state(node, N_ONLINE)
> Index: linux-2.6/mm/hugetlb.c
> ===================================================================
> --- linux-2.6.orig/mm/hugetlb.c 2009-03-19 15:11:49.000000000 -0500
> +++ linux-2.6/mm/hugetlb.c 2009-03-19 15:12:13.000000000 -0500
> @@ -875,7 +875,7 @@
> * can no longer free unreserved surplus pages. This occurs when
> * the nodes with surplus pages have no free pages.
> */
> - unsigned long remaining_iterations = num_online_nodes();
> + unsigned long remaining_iterations = nr_online_nodes;
>
> /* Uncommit the reservation */
> h->resv_huge_pages -= unused_resv_pages;
> @@ -904,7 +904,7 @@
> h->surplus_huge_pages--;
> h->surplus_huge_pages_node[nid]--;
> nr_pages--;
> - remaining_iterations = num_online_nodes();
> + remaining_iterations = nr_online_nodes;
> }
> }
> }
> Index: linux-2.6/mm/page_alloc.c
> ===================================================================
> --- linux-2.6.orig/mm/page_alloc.c 2009-03-19 15:12:19.000000000 -0500
> +++ linux-2.6/mm/page_alloc.c 2009-03-19 15:30:21.000000000 -0500
> @@ -168,6 +168,10 @@
> #if MAX_NUMNODES > 1
> int nr_node_ids __read_mostly = MAX_NUMNODES;
> EXPORT_SYMBOL(nr_node_ids);
> +int nr_online_nodes __read_mostly = 1;
> +EXPORT_SYMBOL(nr_online_nodes);
> +int nr_possible_nodes __read_mostly = MAX_NUMNODES;
> +EXPORT_SYMBOL(nr_possible_nodes);
> #endif
>
> int page_group_by_mobility_disabled __read_mostly;
> @@ -2115,7 +2119,7 @@
> }
>
>
> -#define MAX_NODE_LOAD (num_online_nodes())
> +#define MAX_NODE_LOAD nr_online_nodes
> static int node_load[MAX_NUMNODES];
>
> /**
> Index: linux-2.6/mm/slab.c
> ===================================================================
> --- linux-2.6.orig/mm/slab.c 2009-03-19 15:13:45.000000000 -0500
> +++ linux-2.6/mm/slab.c 2009-03-19 15:15:28.000000000 -0500
> @@ -881,7 +881,6 @@
> */
>
> static int use_alien_caches __read_mostly = 1;
> -static int numa_platform __read_mostly = 1;
> static int __init noaliencache_setup(char *s)
> {
> use_alien_caches = 0;
> @@ -1434,9 +1433,8 @@
> int order;
> int node;
>
> - if (num_possible_nodes() == 1) {
> + if (nr_possible_nodes == 1) {
> use_alien_caches = 0;
> - numa_platform = 0;
> }
>
> for (i = 0; i < NUM_INIT_LISTS; i++) {
> @@ -3526,7 +3524,7 @@
> * variable to skip the call, which is mostly likely to be present in
> * the cache.
> */
> - if (numa_platform && cache_free_alien(cachep, objp))
> + if (nr_possible_nodes > 1 && cache_free_alien(cachep, objp))
> return;
>
> if (likely(ac->avail < ac->limit)) {
> Index: linux-2.6/mm/slub.c
> ===================================================================
> --- linux-2.6.orig/mm/slub.c 2009-03-19 15:13:15.000000000 -0500
> +++ linux-2.6/mm/slub.c 2009-03-19 15:13:38.000000000 -0500
> @@ -3648,7 +3648,7 @@
> to_cpumask(l->cpus));
> }
>
> - if (num_online_nodes() > 1 && !nodes_empty(l->nodes) &&
> + if (nr_online_nodes > 1 && !nodes_empty(l->nodes) &&
> len < PAGE_SIZE - 60) {
> len += sprintf(buf + len, " nodes=");
> len += nodelist_scnprintf(buf + len, PAGE_SIZE - len - 50,
> Index: linux-2.6/net/sunrpc/svc.c
> ===================================================================
> --- linux-2.6.orig/net/sunrpc/svc.c 2009-03-19 15:16:21.000000000 -0500
> +++ linux-2.6/net/sunrpc/svc.c 2009-03-19 15:16:51.000000000 -0500
> @@ -124,7 +124,7 @@
> {
> unsigned int node;
>
> - if (num_online_nodes() > 1) {
> + if (nr_online_nodes > 1) {
> /*
> * Actually have multiple NUMA nodes,
> * so split pools on NUMA node boundaries
>

--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/