Re: [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random()
On Fri, Aug 12, 2022 at 10:46:57AM +0530, Aneesh Kumar K.V wrote: > Yury Norov writes: > > > The functions are pretty thin wrappers around find_bit engine, and > > keeping them in c-file prevents compiler from small_const_nbits() > > optimization, which must take place for all systems with MAX_NUMNODES > > less than BITS_PER_LONG (default is 16 for me). > > > > Moving them to header file doesn't blow up the kernel size: > > add/remove: 1/2 grow/shrink: 9/5 up/down: 968/-88 (880) > > > > CC: Andy Shevchenko > > CC: Rasmus Villemoes > > Signed-off-by: Yury Norov > > --- > > MAINTAINERS | 1 - > > include/linux/nodemask.h | 27 ++- > > lib/Makefile | 2 +- > > lib/nodemask.c | 30 -- > > 4 files changed, 23 insertions(+), 37 deletions(-) > > delete mode 100644 lib/nodemask.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 7c0b8f28aa25..19c8d0ef1177 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -3540,7 +3540,6 @@ F:lib/bitmap.c > > F: lib/cpumask.c > > F: lib/find_bit.c > > F: lib/find_bit_benchmark.c > > -F: lib/nodemask.c > > F: lib/test_bitmap.c > > F: tools/include/linux/bitmap.h > > F: tools/include/linux/find.h > > diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h > > index 0f233b76c9ce..48ebe4007955 100644 > > --- a/include/linux/nodemask.h > > +++ b/include/linux/nodemask.h > > @@ -94,6 +94,7 @@ > > #include > > #include > > #include > > +#include > > > > typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } nodemask_t; > > extern nodemask_t _unused_nodemask_arg_; > > @@ -276,7 +277,14 @@ static inline unsigned int __next_node(int n, const > > nodemask_t *srcp) > > * the first node in src if needed. Returns MAX_NUMNODES if src is empty. > > */ > > #define next_node_in(n, src) __next_node_in((n), &(src)) > > -unsigned int __next_node_in(int node, const nodemask_t *srcp); > > +static inline unsigned int __next_node_in(int node, const nodemask_t *srcp) > > +{ > > + unsigned int ret = __next_node(node, srcp); > > + > > + if (ret == MAX_NUMNODES) > > + ret = __first_node(srcp); > > + return ret; > > +} > > > > static inline void init_nodemask_of_node(nodemask_t *mask, int node) > > { > > @@ -493,14 +501,23 @@ static inline int num_node_state(enum node_states > > state) > > > > #endif > > > > +/* > > + * Return the bit number of a random bit set in the nodemask. > > + * (returns NUMA_NO_NODE if nodemask is empty) > > + */ > > +static inline int node_random(const nodemask_t *maskp) > > +{ > > #if defined(CONFIG_NUMA) && (MAX_NUMNODES > 1) > > -extern int node_random(const nodemask_t *maskp); > > + int w, bit = NUMA_NO_NODE; > > + > > + w = nodes_weight(*maskp); > > + if (w) > > + bit = find_nth_bit(maskp->bits, MAX_NUMNODES, get_random_int() > > % w); > > + return bit; > > #else > > -static inline int node_random(const nodemask_t *mask) > > -{ > > return 0; > > -} > > #endif > > +} > > > > #define node_online_mapnode_states[N_ONLINE] > > #define node_possible_map node_states[N_POSSIBLE] > > diff --git a/lib/Makefile b/lib/Makefile > > index f99bf61f8bbc..731cea0342d1 100644 > > --- a/lib/Makefile > > +++ b/lib/Makefile > > @@ -33,7 +33,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \ > > flex_proportions.o ratelimit.o show_mem.o \ > > is_single_threaded.o plist.o decompress.o kobject_uevent.o \ > > earlycpio.o seq_buf.o siphash.o dec_and_lock.o \ > > -nmi_backtrace.o nodemask.o win_minmax.o memcat_p.o \ > > +nmi_backtrace.o win_minmax.o memcat_p.o \ > > buildid.o > > > > lib-$(CONFIG_PRINTK) += dump_stack.o > > diff --git a/lib/nodemask.c b/lib/nodemask.c > > deleted file mode 100644 > > index 7dad4ce8ff59.. > > --- a/lib/nodemask.c > > +++ /dev/null > > @@ -1,30 +0,0 @@ > > -// SPDX-License-Identifier: GPL-2.0 > > -#include > > -#include > > -#include > > - > > -unsigned int __next_node_in(int node, const nodemask_t *srcp) > > -{ > > - unsigned int ret = __next_node(node, srcp); > > - > > - if (ret == MAX_NUMNODES) > > - ret = __first_node(srcp); > > - return ret; > > -} > > -EXPORT_SYMBOL(__next_node_in); > > - > > -#ifdef CONFIG_NUMA > > -/* > > - * Return the bit number of a random bit set in the nodemask. > > - * (returns NUMA_NO_NODE if nodemask is empty) > > - */ > > -int node_random(const nodemask_t *maskp) > > -{ > > - int w, bit = NUMA_NO_NODE; > > - > > - w = nodes_weight(*maskp); > > - if (w) > > - bit = find_nth_bit(maskp->bits, MAX_NUMNODES, get_random_int() > > % w); > > - return bit; > > -} > > -#endif > > -- > > 2.34.1 > > The patch that got merged (36d4b36b69590fed99356a4426c940a253a93800) still > have lib/nodemask.c Thanks Aneesh. I'll send a fix shortly.
Re: [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random()
Yury Norov writes: > The functions are pretty thin wrappers around find_bit engine, and > keeping them in c-file prevents compiler from small_const_nbits() > optimization, which must take place for all systems with MAX_NUMNODES > less than BITS_PER_LONG (default is 16 for me). > > Moving them to header file doesn't blow up the kernel size: > add/remove: 1/2 grow/shrink: 9/5 up/down: 968/-88 (880) > > CC: Andy Shevchenko > CC: Rasmus Villemoes > Signed-off-by: Yury Norov > --- > MAINTAINERS | 1 - > include/linux/nodemask.h | 27 ++- > lib/Makefile | 2 +- > lib/nodemask.c | 30 -- > 4 files changed, 23 insertions(+), 37 deletions(-) > delete mode 100644 lib/nodemask.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7c0b8f28aa25..19c8d0ef1177 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3540,7 +3540,6 @@ F: lib/bitmap.c > F: lib/cpumask.c > F: lib/find_bit.c > F: lib/find_bit_benchmark.c > -F: lib/nodemask.c > F: lib/test_bitmap.c > F: tools/include/linux/bitmap.h > F: tools/include/linux/find.h > diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h > index 0f233b76c9ce..48ebe4007955 100644 > --- a/include/linux/nodemask.h > +++ b/include/linux/nodemask.h > @@ -94,6 +94,7 @@ > #include > #include > #include > +#include > > typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } nodemask_t; > extern nodemask_t _unused_nodemask_arg_; > @@ -276,7 +277,14 @@ static inline unsigned int __next_node(int n, const > nodemask_t *srcp) > * the first node in src if needed. Returns MAX_NUMNODES if src is empty. > */ > #define next_node_in(n, src) __next_node_in((n), &(src)) > -unsigned int __next_node_in(int node, const nodemask_t *srcp); > +static inline unsigned int __next_node_in(int node, const nodemask_t *srcp) > +{ > + unsigned int ret = __next_node(node, srcp); > + > + if (ret == MAX_NUMNODES) > + ret = __first_node(srcp); > + return ret; > +} > > static inline void init_nodemask_of_node(nodemask_t *mask, int node) > { > @@ -493,14 +501,23 @@ static inline int num_node_state(enum node_states state) > > #endif > > +/* > + * Return the bit number of a random bit set in the nodemask. > + * (returns NUMA_NO_NODE if nodemask is empty) > + */ > +static inline int node_random(const nodemask_t *maskp) > +{ > #if defined(CONFIG_NUMA) && (MAX_NUMNODES > 1) > -extern int node_random(const nodemask_t *maskp); > + int w, bit = NUMA_NO_NODE; > + > + w = nodes_weight(*maskp); > + if (w) > + bit = find_nth_bit(maskp->bits, MAX_NUMNODES, get_random_int() > % w); > + return bit; > #else > -static inline int node_random(const nodemask_t *mask) > -{ > return 0; > -} > #endif > +} > > #define node_online_map node_states[N_ONLINE] > #define node_possible_mapnode_states[N_POSSIBLE] > diff --git a/lib/Makefile b/lib/Makefile > index f99bf61f8bbc..731cea0342d1 100644 > --- a/lib/Makefile > +++ b/lib/Makefile > @@ -33,7 +33,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \ >flex_proportions.o ratelimit.o show_mem.o \ >is_single_threaded.o plist.o decompress.o kobject_uevent.o \ >earlycpio.o seq_buf.o siphash.o dec_and_lock.o \ > - nmi_backtrace.o nodemask.o win_minmax.o memcat_p.o \ > + nmi_backtrace.o win_minmax.o memcat_p.o \ >buildid.o > > lib-$(CONFIG_PRINTK) += dump_stack.o > diff --git a/lib/nodemask.c b/lib/nodemask.c > deleted file mode 100644 > index 7dad4ce8ff59.. > --- a/lib/nodemask.c > +++ /dev/null > @@ -1,30 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0 > -#include > -#include > -#include > - > -unsigned int __next_node_in(int node, const nodemask_t *srcp) > -{ > - unsigned int ret = __next_node(node, srcp); > - > - if (ret == MAX_NUMNODES) > - ret = __first_node(srcp); > - return ret; > -} > -EXPORT_SYMBOL(__next_node_in); > - > -#ifdef CONFIG_NUMA > -/* > - * Return the bit number of a random bit set in the nodemask. > - * (returns NUMA_NO_NODE if nodemask is empty) > - */ > -int node_random(const nodemask_t *maskp) > -{ > - int w, bit = NUMA_NO_NODE; > - > - w = nodes_weight(*maskp); > - if (w) > - bit = find_nth_bit(maskp->bits, MAX_NUMNODES, get_random_int() > % w); > - return bit; > -} > -#endif > -- > 2.34.1 The patch that got merged (36d4b36b69590fed99356a4426c940a253a93800) still have lib/nodemask.c
[RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random()
The functions are pretty thin wrappers around find_bit engine, and keeping them in c-file prevents compiler from small_const_nbits() optimization, which must take place for all systems with MAX_NUMNODES less than BITS_PER_LONG (default is 16 for me). Moving them to header file doesn't blow up the kernel size: add/remove: 1/2 grow/shrink: 9/5 up/down: 968/-88 (880) CC: Andy Shevchenko CC: Rasmus Villemoes Signed-off-by: Yury Norov --- MAINTAINERS | 1 - include/linux/nodemask.h | 27 ++- lib/Makefile | 2 +- lib/nodemask.c | 30 -- 4 files changed, 23 insertions(+), 37 deletions(-) delete mode 100644 lib/nodemask.c diff --git a/MAINTAINERS b/MAINTAINERS index 7c0b8f28aa25..19c8d0ef1177 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3540,7 +3540,6 @@ F:lib/bitmap.c F: lib/cpumask.c F: lib/find_bit.c F: lib/find_bit_benchmark.c -F: lib/nodemask.c F: lib/test_bitmap.c F: tools/include/linux/bitmap.h F: tools/include/linux/find.h diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h index 0f233b76c9ce..48ebe4007955 100644 --- a/include/linux/nodemask.h +++ b/include/linux/nodemask.h @@ -94,6 +94,7 @@ #include #include #include +#include typedef struct { DECLARE_BITMAP(bits, MAX_NUMNODES); } nodemask_t; extern nodemask_t _unused_nodemask_arg_; @@ -276,7 +277,14 @@ static inline unsigned int __next_node(int n, const nodemask_t *srcp) * the first node in src if needed. Returns MAX_NUMNODES if src is empty. */ #define next_node_in(n, src) __next_node_in((n), &(src)) -unsigned int __next_node_in(int node, const nodemask_t *srcp); +static inline unsigned int __next_node_in(int node, const nodemask_t *srcp) +{ + unsigned int ret = __next_node(node, srcp); + + if (ret == MAX_NUMNODES) + ret = __first_node(srcp); + return ret; +} static inline void init_nodemask_of_node(nodemask_t *mask, int node) { @@ -493,14 +501,23 @@ static inline int num_node_state(enum node_states state) #endif +/* + * Return the bit number of a random bit set in the nodemask. + * (returns NUMA_NO_NODE if nodemask is empty) + */ +static inline int node_random(const nodemask_t *maskp) +{ #if defined(CONFIG_NUMA) && (MAX_NUMNODES > 1) -extern int node_random(const nodemask_t *maskp); + int w, bit = NUMA_NO_NODE; + + w = nodes_weight(*maskp); + if (w) + bit = find_nth_bit(maskp->bits, MAX_NUMNODES, get_random_int() % w); + return bit; #else -static inline int node_random(const nodemask_t *mask) -{ return 0; -} #endif +} #define node_online_mapnode_states[N_ONLINE] #define node_possible_map node_states[N_POSSIBLE] diff --git a/lib/Makefile b/lib/Makefile index f99bf61f8bbc..731cea0342d1 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -33,7 +33,7 @@ lib-y := ctype.o string.o vsprintf.o cmdline.o \ flex_proportions.o ratelimit.o show_mem.o \ is_single_threaded.o plist.o decompress.o kobject_uevent.o \ earlycpio.o seq_buf.o siphash.o dec_and_lock.o \ -nmi_backtrace.o nodemask.o win_minmax.o memcat_p.o \ +nmi_backtrace.o win_minmax.o memcat_p.o \ buildid.o lib-$(CONFIG_PRINTK) += dump_stack.o diff --git a/lib/nodemask.c b/lib/nodemask.c deleted file mode 100644 index 7dad4ce8ff59.. --- a/lib/nodemask.c +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include -#include -#include - -unsigned int __next_node_in(int node, const nodemask_t *srcp) -{ - unsigned int ret = __next_node(node, srcp); - - if (ret == MAX_NUMNODES) - ret = __first_node(srcp); - return ret; -} -EXPORT_SYMBOL(__next_node_in); - -#ifdef CONFIG_NUMA -/* - * Return the bit number of a random bit set in the nodemask. - * (returns NUMA_NO_NODE if nodemask is empty) - */ -int node_random(const nodemask_t *maskp) -{ - int w, bit = NUMA_NO_NODE; - - w = nodes_weight(*maskp); - if (w) - bit = find_nth_bit(maskp->bits, MAX_NUMNODES, get_random_int() % w); - return bit; -} -#endif -- 2.34.1