nodemask.h: change any_online_node() to a static inline
PATCH 1/1 nodemask.h: change any_online_node() to a static inline
Change any_online_node() from a macro to a static inline function.
The macro any_online_node() uses the local symbol 'node', this causes a sparse warning in net/sunrpc/svc.c. Changing the macro to a static inline function quites the sparse warning and will prevent the same warning from possibly occurring in the future.
---
include/linux/nodemask.h +11 -9
@@ -467,15 +467,6 @@ static inline int num_node_state(enum node_states state)#define node_online_map node_states[N_ONLINE]#define node_possible_map node_states[N_POSSIBLE] -#define any_online_node (mask) \ -({ \ - int node; \ - for_each_node_mask(node, (mask)) \ - if (node_online(node)) \ - break; \ - node; \ -}) -#define num_online_nodes () num_node_state(N_ONLINE)#define num_possible_nodes () num_node_state(N_POSSIBLE)#define node_online (node) node_state((node), N_ONLINE)
@@ -484,6 +475,17 @@ static inline int num_node_state(enum node_states state)#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) +static inline int any_online_node (nodemask_t mask) +{ +int node; + +for_each_node_mask (node, mask) { +if (node_online (node)) +break ; + } +return node; +} +/* * For nodemask scrach area. * NODEMASK_ALLOC(type, name) allocates an object with a specified type and
Quoted 28 lines · H Hartley Sweeten
include/linux/nodemask.h +4
-9@@ -467,15 +467,6 @@ static inline int num_node_state(enum node_states state)#define node_online_map node_states[N_ONLINE]#define node_possible_map node_states[N_POSSIBLE] -#define any_online_node (mask) \ -({ \ - int node; \ - for_each_node_mask(node, (mask)) \ - if (node_online(node)) \ - break; \ - node; \ -}) -#define num_online_nodes () num_node_state(N_ONLINE)#define num_possible_nodes () num_node_state(N_POSSIBLE)#define node_online (node) node_state((node), N_ONLINE)@@ -484,6 +475,17 @@ static inline int num_node_state(enum node_states state)#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) +static inline int any_online_node (nodemask_t mask) +{ +int node; +
include/linux/nodemask.h static inline int num_node_state(enum node_states state) · line 491
Quoted 10 lines
include/linux/nodemask.h +7
-0+for_each_node_mask (node, mask) { +if (node_online (node)) +break ; + } +return node; +} +/* * For nodemask scrach area. * NODEMASK_ALLOC(type, name) allocates an object with a specified type and
Could you simply remove any_online_node() and replace its callers with first_node(node_online_map) instead?
Quoted 35 lines · David Rientjes
Quoted 33 lines · H Hartley Sweeten
include/linux/nodemask.h +9
-9@@ -467,15 +467,6 @@ static inline int num_node_state(enum node_states state)#define node_online_map node_states[N_ONLINE]#define node_possible_map node_states[N_POSSIBLE] -#define any_online_node (mask) \ -({ \ - int node; \ - for_each_node_mask(node, (mask)) \ - if (node_online(node)) \ - break; \ - node; \ -}) -#define num_online_nodes () num_node_state(N_ONLINE)#define num_possible_nodes () num_node_state(N_POSSIBLE)#define node_online (node) node_state((node), N_ONLINE)@@ -484,6 +475,17 @@ static inline int num_node_state(enum node_states state)#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) +static inline int any_online_node (nodemask_t mask) +{ +int node; + +for_each_node_mask (node, mask) { +if (node_online (node)) +break ; + } +return node;
Quoted 8 lines
include/linux/nodemask.h static inline int num_node_state(enum node_states state) · line 491
Quoted 5 lines
include/linux/nodemask.h +2
-0+} +/* * For nodemask scrach area. * NODEMASK_ALLOC(type, name) allocates an object with a specified type and Could you simply remove
any_online_node()and replace its callers withfirst_node(node_online_map)instead?
No idea ;-)
My main intention with this patch was to quite some sparse noise in net/sunrpc/svc.c. This seemed the cleanest way to do it.
It looks like any_online_node is, currently, only called by:
arch/powerpc/mm/numa.c
3 places, twice setting a local int variable called 'nid' and once just returning the 'node' from the macro. All three call the marco as:
... any_online_node(NODE_MASK_ALL);
net/sunrpc/svc.c
1 place, setting a local unsigned int variable called 'node'. Calls the macro as:
... any_online_node(node_online_map);
I really don't know if the macro could be replaced by first_node(node_online_map).
Regards,
Hartley
Quoted 8 lines · H Hartley Sweeten
Quoted 2 lines
Could you simply remove
any_online_node()and replace its callers withfirst_node(node_online_map)instead?No idea ;-)
My main intention with this patch was to quite some sparse noise in
net/sunrpc/svc.c. This seemed the cleanest way to do it.
It certainly silents the warning, which is good, but it also leaves behind a macro that can simply be removed and reimplemented with first_node(node_online_map) given the current use cases.
Quoted 8 lines
It looks like
any_online_nodeis, currently, only called by:
arch/powerpc/mm/numa.c
3 places, twice setting a local int variable called 'nid' and once just returning the 'node' from the macro. All three call the marco as:... any_online_node(NODE_MASK_ALL);
NODE_MASK_ALL is a nodemask_t with all bits set, so this will simply return the first nid where node_online(nid) is true. That's equivalent to first_node(node_online_map).
Quoted 6 lines
net/sunrpc/svc.c
1 place, setting a local unsigned int variable called 'node'. Calls the macro as:... any_online_node(node_online_map);
Same thing, except this doesn't require any iteration since node_online() for the first bit in node_online_map will always be true.
Quoted 2 lines
I really don't know if the macro could be replaced by
first_node(node_online_map).
It can, so I think an even better step would be to remove any_online_node() completely and convert the callers you've identified to use the new implementation.
Thanks!