* [PATCH 0/2] lib/nodemask: inline wrappers around bitmap @ 2022-07-23 21:45 Yury Norov 2022-07-23 21:45 ` [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h Yury Norov 2022-07-23 21:45 ` [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random() Yury Norov 0 siblings, 2 replies; 29+ messages in thread From: Yury Norov @ 2022-07-23 21:45 UTC (permalink / raw) To: linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell Cc: Yury Norov On top of git@github.com:/norov/linux.git bitmap-for-next. There are just 2 functions in nodemask.c, both are thin wrappers around bitmap API. 1st patch of this series drops dependency on <asm/machdep.h for powerpc, which prevents from inlining those nodemask functions, and 2nd patch inlines them and drops nodemask.c. Yury Norov (2): powerpc: drop dependency on <asm/machdep.h> in archrandom.h lib/nodemask: inline next_node_in() and node_random() MAINTAINERS | 1 - arch/powerpc/include/asm/archrandom.h | 9 +------- arch/powerpc/kernel/setup-common.c | 11 ++++++++++ include/linux/nodemask.h | 27 +++++++++++++++++++----- lib/Makefile | 2 +- lib/nodemask.c | 30 --------------------------- 6 files changed, 35 insertions(+), 45 deletions(-) delete mode 100644 lib/nodemask.c -- 2.34.1 ^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-23 21:45 [PATCH 0/2] lib/nodemask: inline wrappers around bitmap Yury Norov @ 2022-07-23 21:45 ` Yury Norov 2022-07-25 7:28 ` Andy Shevchenko ` (2 more replies) 2022-07-23 21:45 ` [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random() Yury Norov 1 sibling, 3 replies; 29+ messages in thread From: Yury Norov @ 2022-07-23 21:45 UTC (permalink / raw) To: linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell Cc: Yury Norov archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes circular header dependency, if generic nodemask.h includes random.h: In file included from include/linux/cred.h:16, from include/linux/seq_file.h:13, from arch/powerpc/include/asm/machdep.h:6, from arch/powerpc/include/asm/archrandom.h:5, from include/linux/random.h:109, from include/linux/nodemask.h:97, from include/linux/list_lru.h:12, from include/linux/fs.h:13, from include/linux/compat.h:17, from arch/powerpc/kernel/asm-offsets.c:12: include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' 1203 | nodemask_t mems_allowed; | ^~~~~~~~~~ Fix it by removing <asm/machdep.h> dependency from archrandom.h Signed-off-by: Yury Norov <yury.norov@gmail.com> --- arch/powerpc/include/asm/archrandom.h | 9 +-------- arch/powerpc/kernel/setup-common.c | 11 +++++++++++ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h index 9a53e29680f4..21def59ef1a6 100644 --- a/arch/powerpc/include/asm/archrandom.h +++ b/arch/powerpc/include/asm/archrandom.h @@ -4,7 +4,7 @@ #ifdef CONFIG_ARCH_RANDOM -#include <asm/machdep.h> +bool __must_check arch_get_random_seed_long(unsigned long *v); static inline bool __must_check arch_get_random_long(unsigned long *v) { @@ -16,13 +16,6 @@ static inline bool __must_check arch_get_random_int(unsigned int *v) return false; } -static inline bool __must_check arch_get_random_seed_long(unsigned long *v) -{ - if (ppc_md.get_random_seed) - return ppc_md.get_random_seed(v); - - return false; -} static inline bool __must_check arch_get_random_seed_int(unsigned int *v) { diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c index eb0077b302e2..18c5fa5918bf 100644 --- a/arch/powerpc/kernel/setup-common.c +++ b/arch/powerpc/kernel/setup-common.c @@ -171,6 +171,17 @@ EXPORT_SYMBOL_GPL(machine_power_off); void (*pm_power_off)(void); EXPORT_SYMBOL_GPL(pm_power_off); +#ifdef CONFIG_ARCH_RANDOM +bool __must_check arch_get_random_seed_long(unsigned long *v) +{ + if (ppc_md.get_random_seed) + return ppc_md.get_random_seed(v); + + return false; +} +EXPORT_SYMBOL(arch_get_random_seed_long); +#endif + void machine_halt(void) { machine_shutdown(); -- 2.34.1 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-23 21:45 ` [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h Yury Norov @ 2022-07-25 7:28 ` Andy Shevchenko 2022-07-25 8:34 ` Michael Ellerman 2022-07-25 12:22 ` Michael Ellerman 2 siblings, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2022-07-25 7:28 UTC (permalink / raw) To: Yury Norov Cc: Stephen Rothwell, Rasmus Villemoes, Linux Kernel Mailing List, Paul Mackerras, Andy Shevchenko, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > circular header dependency, if generic nodemask.h includes random.h: > > In file included from include/linux/cred.h:16, > from include/linux/seq_file.h:13, > from arch/powerpc/include/asm/machdep.h:6, > from arch/powerpc/include/asm/archrandom.h:5, > from include/linux/random.h:109, > from include/linux/nodemask.h:97, > from include/linux/list_lru.h:12, > from include/linux/fs.h:13, > from include/linux/compat.h:17, > from arch/powerpc/kernel/asm-offsets.c:12: > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > 1203 | nodemask_t mems_allowed; > | ^~~~~~~~~~ > > Fix it by removing <asm/machdep.h> dependency from archrandom.h ... > EXPORT_SYMBOL_GPL(pm_power_off); ^^^ (Note this and read below) ... > +EXPORT_SYMBOL(arch_get_random_seed_long); It can't be like this. Brief browsing of the callees shows that. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-25 7:28 ` Andy Shevchenko 0 siblings, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2022-07-25 7:28 UTC (permalink / raw) To: Yury Norov Cc: Linux Kernel Mailing List, Andy Shevchenko, Rasmus Villemoes, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > circular header dependency, if generic nodemask.h includes random.h: > > In file included from include/linux/cred.h:16, > from include/linux/seq_file.h:13, > from arch/powerpc/include/asm/machdep.h:6, > from arch/powerpc/include/asm/archrandom.h:5, > from include/linux/random.h:109, > from include/linux/nodemask.h:97, > from include/linux/list_lru.h:12, > from include/linux/fs.h:13, > from include/linux/compat.h:17, > from arch/powerpc/kernel/asm-offsets.c:12: > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > 1203 | nodemask_t mems_allowed; > | ^~~~~~~~~~ > > Fix it by removing <asm/machdep.h> dependency from archrandom.h ... > EXPORT_SYMBOL_GPL(pm_power_off); ^^^ (Note this and read below) ... > +EXPORT_SYMBOL(arch_get_random_seed_long); It can't be like this. Brief browsing of the callees shows that. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-25 7:28 ` Andy Shevchenko @ 2022-07-25 16:17 ` Yury Norov -1 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-07-25 16:17 UTC (permalink / raw) To: Andy Shevchenko Cc: Stephen Rothwell, Rasmus Villemoes, Linux Kernel Mailing List, Paul Mackerras, Andy Shevchenko, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > > > > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > > circular header dependency, if generic nodemask.h includes random.h: > > > > In file included from include/linux/cred.h:16, > > from include/linux/seq_file.h:13, > > from arch/powerpc/include/asm/machdep.h:6, > > from arch/powerpc/include/asm/archrandom.h:5, > > from include/linux/random.h:109, > > from include/linux/nodemask.h:97, > > from include/linux/list_lru.h:12, > > from include/linux/fs.h:13, > > from include/linux/compat.h:17, > > from arch/powerpc/kernel/asm-offsets.c:12: > > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > > 1203 | nodemask_t mems_allowed; > > | ^~~~~~~~~~ > > > > Fix it by removing <asm/machdep.h> dependency from archrandom.h > > ... > > > EXPORT_SYMBOL_GPL(pm_power_off); > > ^^^ (Note this and read below) > > ... > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > It can't be like this. Brief browsing of the callees shows that. Is my understanding correct that you're suggesting to make it GPL? ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, so it's available for non-GPL code now. I don't want to change it. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-25 16:17 ` Yury Norov 0 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-07-25 16:17 UTC (permalink / raw) To: Andy Shevchenko Cc: Linux Kernel Mailing List, Andy Shevchenko, Rasmus Villemoes, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > > > > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > > circular header dependency, if generic nodemask.h includes random.h: > > > > In file included from include/linux/cred.h:16, > > from include/linux/seq_file.h:13, > > from arch/powerpc/include/asm/machdep.h:6, > > from arch/powerpc/include/asm/archrandom.h:5, > > from include/linux/random.h:109, > > from include/linux/nodemask.h:97, > > from include/linux/list_lru.h:12, > > from include/linux/fs.h:13, > > from include/linux/compat.h:17, > > from arch/powerpc/kernel/asm-offsets.c:12: > > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > > 1203 | nodemask_t mems_allowed; > > | ^~~~~~~~~~ > > > > Fix it by removing <asm/machdep.h> dependency from archrandom.h > > ... > > > EXPORT_SYMBOL_GPL(pm_power_off); > > ^^^ (Note this and read below) > > ... > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > It can't be like this. Brief browsing of the callees shows that. Is my understanding correct that you're suggesting to make it GPL? ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, so it's available for non-GPL code now. I don't want to change it. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-25 16:17 ` Yury Norov @ 2022-07-25 21:39 ` Andy Shevchenko -1 siblings, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2022-07-25 21:39 UTC (permalink / raw) To: Yury Norov Cc: Stephen Rothwell, Rasmus Villemoes, Linux Kernel Mailing List, Paul Mackerras, Andy Shevchenko, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT On Mon, Jul 25, 2022 at 6:19 PM Yury Norov <yury.norov@gmail.com> wrote: > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: ... > > > EXPORT_SYMBOL_GPL(pm_power_off); > > > > ^^^ (Note this and read below) > > > > ... > > > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > > > It can't be like this. Brief browsing of the callees shows that. > > Is my understanding correct that you're suggesting to make it GPL? > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > so it's available for non-GPL code now. I don't want to change it. The symbols your function calls are GPL. As far as I understand (not a lawyer!) it logically one may not call GPL and pretend to be non-GPL. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-25 21:39 ` Andy Shevchenko 0 siblings, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2022-07-25 21:39 UTC (permalink / raw) To: Yury Norov Cc: Linux Kernel Mailing List, Andy Shevchenko, Rasmus Villemoes, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell On Mon, Jul 25, 2022 at 6:19 PM Yury Norov <yury.norov@gmail.com> wrote: > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: ... > > > EXPORT_SYMBOL_GPL(pm_power_off); > > > > ^^^ (Note this and read below) > > > > ... > > > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > > > It can't be like this. Brief browsing of the callees shows that. > > Is my understanding correct that you're suggesting to make it GPL? > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > so it's available for non-GPL code now. I don't want to change it. The symbols your function calls are GPL. As far as I understand (not a lawyer!) it logically one may not call GPL and pretend to be non-GPL. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-25 21:39 ` Andy Shevchenko @ 2022-07-25 23:32 ` Yury Norov -1 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-07-25 23:32 UTC (permalink / raw) To: Andy Shevchenko Cc: Stephen Rothwell, Rasmus Villemoes, Linux Kernel Mailing List, Paul Mackerras, Andy Shevchenko, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT On Mon, Jul 25, 2022 at 11:39:39PM +0200, Andy Shevchenko wrote: > On Mon, Jul 25, 2022 at 6:19 PM Yury Norov <yury.norov@gmail.com> wrote: > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > > ... > > > > > EXPORT_SYMBOL_GPL(pm_power_off); > > > > > > ^^^ (Note this and read below) > > > > > > ... > > > > > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > > > > > It can't be like this. Brief browsing of the callees shows that. > > > > Is my understanding correct that you're suggesting to make it GPL? > > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > > so it's available for non-GPL code now. I don't want to change it. > > The symbols your function calls are GPL. As far as I understand (not a > lawyer!) it logically one may not call GPL and pretend to be non-GPL. Can you explain what you mean in details? The function is: static inline bool __must_check arch_get_random_seed_long(unsigned long *v) { if (ppc_md.get_random_seed) return ppc_md.get_random_seed(v); return false; } ppc_md is non-GPL: 77 /* The main machine-dep calls structure 78 */ 79 struct machdep_calls ppc_md; 80 EXPORT_SYMBOL(ppc_md); And get_random_seed is initialized in in arch/powerpc/platforms/powernv/rng.c with different functions that are static and not exported at all. I don't understand where arch_get_random_seed_long calls GPL... ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-25 23:32 ` Yury Norov 0 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-07-25 23:32 UTC (permalink / raw) To: Andy Shevchenko Cc: Linux Kernel Mailing List, Andy Shevchenko, Rasmus Villemoes, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell On Mon, Jul 25, 2022 at 11:39:39PM +0200, Andy Shevchenko wrote: > On Mon, Jul 25, 2022 at 6:19 PM Yury Norov <yury.norov@gmail.com> wrote: > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > > ... > > > > > EXPORT_SYMBOL_GPL(pm_power_off); > > > > > > ^^^ (Note this and read below) > > > > > > ... > > > > > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > > > > > It can't be like this. Brief browsing of the callees shows that. > > > > Is my understanding correct that you're suggesting to make it GPL? > > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > > so it's available for non-GPL code now. I don't want to change it. > > The symbols your function calls are GPL. As far as I understand (not a > lawyer!) it logically one may not call GPL and pretend to be non-GPL. Can you explain what you mean in details? The function is: static inline bool __must_check arch_get_random_seed_long(unsigned long *v) { if (ppc_md.get_random_seed) return ppc_md.get_random_seed(v); return false; } ppc_md is non-GPL: 77 /* The main machine-dep calls structure 78 */ 79 struct machdep_calls ppc_md; 80 EXPORT_SYMBOL(ppc_md); And get_random_seed is initialized in in arch/powerpc/platforms/powernv/rng.c with different functions that are static and not exported at all. I don't understand where arch_get_random_seed_long calls GPL... ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-25 23:32 ` Yury Norov @ 2022-07-26 6:13 ` Andy Shevchenko -1 siblings, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2022-07-26 6:13 UTC (permalink / raw) To: Yury Norov Cc: Stephen Rothwell, Rasmus Villemoes, Linux Kernel Mailing List, Paul Mackerras, Andy Shevchenko, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT On Tue, Jul 26, 2022 at 1:35 AM Yury Norov <yury.norov@gmail.com> wrote: > On Mon, Jul 25, 2022 at 11:39:39PM +0200, Andy Shevchenko wrote: > > On Mon, Jul 25, 2022 at 6:19 PM Yury Norov <yury.norov@gmail.com> wrote: > > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > > > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: ... > > > > > EXPORT_SYMBOL_GPL(pm_power_off); > > > > > > > > ^^^ (Note this and read below) > > > > > > > > ... > > > > > > > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > > > > > > > It can't be like this. Brief browsing of the callees shows that. > > > > > > Is my understanding correct that you're suggesting to make it GPL? > > > > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > > > so it's available for non-GPL code now. I don't want to change it. > > > > The symbols your function calls are GPL. As far as I understand (not a > > lawyer!) it logically one may not call GPL and pretend to be non-GPL. > > Can you explain what you mean in details? > > The function is: > static inline bool __must_check arch_get_random_seed_long(unsigned long *v) > { > if (ppc_md.get_random_seed) > return ppc_md.get_random_seed(v); > > return false; > } > > ppc_md is non-GPL: > 77 /* The main machine-dep calls structure > 78 */ > 79 struct machdep_calls ppc_md; > 80 EXPORT_SYMBOL(ppc_md); What a mess... > And get_random_seed is initialized in in arch/powerpc/platforms/powernv/rng.c > with different functions that are static and not exported at all. > > I don't understand where arch_get_random_seed_long calls GPL... The ->get_random_seed() (aka "callees" in my previous mail) are all GPL (maybe I missed one out of five which is non-GPL, but then it's even more of a mess). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-26 6:13 ` Andy Shevchenko 0 siblings, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2022-07-26 6:13 UTC (permalink / raw) To: Yury Norov Cc: Linux Kernel Mailing List, Andy Shevchenko, Rasmus Villemoes, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell On Tue, Jul 26, 2022 at 1:35 AM Yury Norov <yury.norov@gmail.com> wrote: > On Mon, Jul 25, 2022 at 11:39:39PM +0200, Andy Shevchenko wrote: > > On Mon, Jul 25, 2022 at 6:19 PM Yury Norov <yury.norov@gmail.com> wrote: > > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > > > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: ... > > > > > EXPORT_SYMBOL_GPL(pm_power_off); > > > > > > > > ^^^ (Note this and read below) > > > > > > > > ... > > > > > > > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > > > > > > > It can't be like this. Brief browsing of the callees shows that. > > > > > > Is my understanding correct that you're suggesting to make it GPL? > > > > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > > > so it's available for non-GPL code now. I don't want to change it. > > > > The symbols your function calls are GPL. As far as I understand (not a > > lawyer!) it logically one may not call GPL and pretend to be non-GPL. > > Can you explain what you mean in details? > > The function is: > static inline bool __must_check arch_get_random_seed_long(unsigned long *v) > { > if (ppc_md.get_random_seed) > return ppc_md.get_random_seed(v); > > return false; > } > > ppc_md is non-GPL: > 77 /* The main machine-dep calls structure > 78 */ > 79 struct machdep_calls ppc_md; > 80 EXPORT_SYMBOL(ppc_md); What a mess... > And get_random_seed is initialized in in arch/powerpc/platforms/powernv/rng.c > with different functions that are static and not exported at all. > > I don't understand where arch_get_random_seed_long calls GPL... The ->get_random_seed() (aka "callees" in my previous mail) are all GPL (maybe I missed one out of five which is non-GPL, but then it's even more of a mess). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-26 6:13 ` Andy Shevchenko @ 2022-07-26 6:15 ` Andy Shevchenko -1 siblings, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2022-07-26 6:15 UTC (permalink / raw) To: Yury Norov Cc: Stephen Rothwell, Rasmus Villemoes, Linux Kernel Mailing List, Paul Mackerras, Andy Shevchenko, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT On Tue, Jul 26, 2022 at 8:13 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Tue, Jul 26, 2022 at 1:35 AM Yury Norov <yury.norov@gmail.com> wrote: > > On Mon, Jul 25, 2022 at 11:39:39PM +0200, Andy Shevchenko wrote: > > > On Mon, Jul 25, 2022 at 6:19 PM Yury Norov <yury.norov@gmail.com> wrote: > > > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > > > > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > > ... > > > > > > > EXPORT_SYMBOL_GPL(pm_power_off); > > > > > > > > > > ^^^ (Note this and read below) > > > > > > > > > > ... > > > > > > > > > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > > > > > > > > > It can't be like this. Brief browsing of the callees shows that. > > > > > > > > Is my understanding correct that you're suggesting to make it GPL? > > > > > > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > > > > so it's available for non-GPL code now. I don't want to change it. > > > > > > The symbols your function calls are GPL. As far as I understand (not a > > > lawyer!) it logically one may not call GPL and pretend to be non-GPL. > > > > Can you explain what you mean in details? > > > > The function is: > > static inline bool __must_check arch_get_random_seed_long(unsigned long *v) > > { > > if (ppc_md.get_random_seed) > > return ppc_md.get_random_seed(v); > > > > return false; > > } > > > > ppc_md is non-GPL: > > 77 /* The main machine-dep calls structure > > 78 */ > > 79 struct machdep_calls ppc_md; > > 80 EXPORT_SYMBOL(ppc_md); > > What a mess... > > > And get_random_seed is initialized in in arch/powerpc/platforms/powernv/rng.c > > with different functions that are static and not exported at all. To be clear, their license is defined in the file: "GPL-2.0-or-later". But again, not a lawyer, just using my common sense. > > I don't understand where arch_get_random_seed_long calls GPL... > > The ->get_random_seed() (aka "callees" in my previous mail) are all > GPL (maybe I missed one out of five which is non-GPL, but then it's > even more of a mess). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-26 6:15 ` Andy Shevchenko 0 siblings, 0 replies; 29+ messages in thread From: Andy Shevchenko @ 2022-07-26 6:15 UTC (permalink / raw) To: Yury Norov Cc: Linux Kernel Mailing List, Andy Shevchenko, Rasmus Villemoes, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell On Tue, Jul 26, 2022 at 8:13 AM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Tue, Jul 26, 2022 at 1:35 AM Yury Norov <yury.norov@gmail.com> wrote: > > On Mon, Jul 25, 2022 at 11:39:39PM +0200, Andy Shevchenko wrote: > > > On Mon, Jul 25, 2022 at 6:19 PM Yury Norov <yury.norov@gmail.com> wrote: > > > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > > > > > On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > > ... > > > > > > > EXPORT_SYMBOL_GPL(pm_power_off); > > > > > > > > > > ^^^ (Note this and read below) > > > > > > > > > > ... > > > > > > > > > > > +EXPORT_SYMBOL(arch_get_random_seed_long); > > > > > > > > > > It can't be like this. Brief browsing of the callees shows that. > > > > > > > > Is my understanding correct that you're suggesting to make it GPL? > > > > > > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > > > > so it's available for non-GPL code now. I don't want to change it. > > > > > > The symbols your function calls are GPL. As far as I understand (not a > > > lawyer!) it logically one may not call GPL and pretend to be non-GPL. > > > > Can you explain what you mean in details? > > > > The function is: > > static inline bool __must_check arch_get_random_seed_long(unsigned long *v) > > { > > if (ppc_md.get_random_seed) > > return ppc_md.get_random_seed(v); > > > > return false; > > } > > > > ppc_md is non-GPL: > > 77 /* The main machine-dep calls structure > > 78 */ > > 79 struct machdep_calls ppc_md; > > 80 EXPORT_SYMBOL(ppc_md); > > What a mess... > > > And get_random_seed is initialized in in arch/powerpc/platforms/powernv/rng.c > > with different functions that are static and not exported at all. To be clear, their license is defined in the file: "GPL-2.0-or-later". But again, not a lawyer, just using my common sense. > > I don't understand where arch_get_random_seed_long calls GPL... > > The ->get_random_seed() (aka "callees" in my previous mail) are all > GPL (maybe I missed one out of five which is non-GPL, but then it's > even more of a mess). -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-25 16:17 ` Yury Norov @ 2022-07-26 6:57 ` Michael Ellerman -1 siblings, 0 replies; 29+ messages in thread From: Michael Ellerman @ 2022-07-26 6:57 UTC (permalink / raw) To: Yury Norov, Andy Shevchenko Cc: Stephen Rothwell, Jason A. Donenfeld, Rasmus Villemoes, Linux Kernel Mailing List, Paul Mackerras, Andy Shevchenko, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT Yury Norov <yury.norov@gmail.com> writes: > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: >> On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: >> > >> > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes >> > circular header dependency, if generic nodemask.h includes random.h: >> > >> > In file included from include/linux/cred.h:16, >> > from include/linux/seq_file.h:13, >> > from arch/powerpc/include/asm/machdep.h:6, >> > from arch/powerpc/include/asm/archrandom.h:5, >> > from include/linux/random.h:109, >> > from include/linux/nodemask.h:97, >> > from include/linux/list_lru.h:12, >> > from include/linux/fs.h:13, >> > from include/linux/compat.h:17, >> > from arch/powerpc/kernel/asm-offsets.c:12: >> > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' >> > 1203 | nodemask_t mems_allowed; >> > | ^~~~~~~~~~ >> > >> > Fix it by removing <asm/machdep.h> dependency from archrandom.h >> >> ... >> >> > EXPORT_SYMBOL_GPL(pm_power_off); >> >> ^^^ (Note this and read below) >> >> ... >> >> > +EXPORT_SYMBOL(arch_get_random_seed_long); >> >> It can't be like this. Brief browsing of the callees shows that. > > Is my understanding correct that you're suggesting to make it GPL? > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > so it's available for non-GPL code now. I don't want to change it. That's true, your change maintains the status quo. But I think we actually don't need it exported to modules, I think it's a private detail of the RNG <-> architecture interface, not something that modules should be calling. So I think it's OK to drop the EXPORT_SYMBOL, either in this patch or a subsequent one if you don't want to rebase. cheers ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-26 6:57 ` Michael Ellerman 0 siblings, 0 replies; 29+ messages in thread From: Michael Ellerman @ 2022-07-26 6:57 UTC (permalink / raw) To: Yury Norov, Andy Shevchenko Cc: Linux Kernel Mailing List, Andy Shevchenko, Rasmus Villemoes, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell, Jason A. Donenfeld Yury Norov <yury.norov@gmail.com> writes: > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: >> On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: >> > >> > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes >> > circular header dependency, if generic nodemask.h includes random.h: >> > >> > In file included from include/linux/cred.h:16, >> > from include/linux/seq_file.h:13, >> > from arch/powerpc/include/asm/machdep.h:6, >> > from arch/powerpc/include/asm/archrandom.h:5, >> > from include/linux/random.h:109, >> > from include/linux/nodemask.h:97, >> > from include/linux/list_lru.h:12, >> > from include/linux/fs.h:13, >> > from include/linux/compat.h:17, >> > from arch/powerpc/kernel/asm-offsets.c:12: >> > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' >> > 1203 | nodemask_t mems_allowed; >> > | ^~~~~~~~~~ >> > >> > Fix it by removing <asm/machdep.h> dependency from archrandom.h >> >> ... >> >> > EXPORT_SYMBOL_GPL(pm_power_off); >> >> ^^^ (Note this and read below) >> >> ... >> >> > +EXPORT_SYMBOL(arch_get_random_seed_long); >> >> It can't be like this. Brief browsing of the callees shows that. > > Is my understanding correct that you're suggesting to make it GPL? > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > so it's available for non-GPL code now. I don't want to change it. That's true, your change maintains the status quo. But I think we actually don't need it exported to modules, I think it's a private detail of the RNG <-> architecture interface, not something that modules should be calling. So I think it's OK to drop the EXPORT_SYMBOL, either in this patch or a subsequent one if you don't want to rebase. cheers ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-26 6:57 ` Michael Ellerman @ 2022-07-26 15:19 ` Yury Norov -1 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-07-26 15:19 UTC (permalink / raw) To: Michael Ellerman Cc: Stephen Rothwell, Jason A. Donenfeld, Rasmus Villemoes, Linux Kernel Mailing List, Andy Shevchenko, Paul Mackerras, Andy Shevchenko, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT On Tue, Jul 26, 2022 at 04:57:38PM +1000, Michael Ellerman wrote: > Yury Norov <yury.norov@gmail.com> writes: > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > >> On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > >> > > >> > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > >> > circular header dependency, if generic nodemask.h includes random.h: > >> > > >> > In file included from include/linux/cred.h:16, > >> > from include/linux/seq_file.h:13, > >> > from arch/powerpc/include/asm/machdep.h:6, > >> > from arch/powerpc/include/asm/archrandom.h:5, > >> > from include/linux/random.h:109, > >> > from include/linux/nodemask.h:97, > >> > from include/linux/list_lru.h:12, > >> > from include/linux/fs.h:13, > >> > from include/linux/compat.h:17, > >> > from arch/powerpc/kernel/asm-offsets.c:12: > >> > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > >> > 1203 | nodemask_t mems_allowed; > >> > | ^~~~~~~~~~ > >> > > >> > Fix it by removing <asm/machdep.h> dependency from archrandom.h > >> > >> ... > >> > >> > EXPORT_SYMBOL_GPL(pm_power_off); > >> > >> ^^^ (Note this and read below) > >> > >> ... > >> > >> > +EXPORT_SYMBOL(arch_get_random_seed_long); > >> > >> It can't be like this. Brief browsing of the callees shows that. > > > > Is my understanding correct that you're suggesting to make it GPL? > > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > > so it's available for non-GPL code now. I don't want to change it. > > That's true, your change maintains the status quo. > > But I think we actually don't need it exported to modules, I think it's > a private detail of the RNG <-> architecture interface, not something > that modules should be calling. > > So I think it's OK to drop the EXPORT_SYMBOL, either in this patch or a > subsequent one if you don't want to rebase. OK, changed. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-26 15:19 ` Yury Norov 0 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-07-26 15:19 UTC (permalink / raw) To: Michael Ellerman Cc: Andy Shevchenko, Linux Kernel Mailing List, Andy Shevchenko, Rasmus Villemoes, open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell, Jason A. Donenfeld On Tue, Jul 26, 2022 at 04:57:38PM +1000, Michael Ellerman wrote: > Yury Norov <yury.norov@gmail.com> writes: > > On Mon, Jul 25, 2022 at 09:28:12AM +0200, Andy Shevchenko wrote: > >> On Sun, Jul 24, 2022 at 12:19 AM Yury Norov <yury.norov@gmail.com> wrote: > >> > > >> > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > >> > circular header dependency, if generic nodemask.h includes random.h: > >> > > >> > In file included from include/linux/cred.h:16, > >> > from include/linux/seq_file.h:13, > >> > from arch/powerpc/include/asm/machdep.h:6, > >> > from arch/powerpc/include/asm/archrandom.h:5, > >> > from include/linux/random.h:109, > >> > from include/linux/nodemask.h:97, > >> > from include/linux/list_lru.h:12, > >> > from include/linux/fs.h:13, > >> > from include/linux/compat.h:17, > >> > from arch/powerpc/kernel/asm-offsets.c:12: > >> > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > >> > 1203 | nodemask_t mems_allowed; > >> > | ^~~~~~~~~~ > >> > > >> > Fix it by removing <asm/machdep.h> dependency from archrandom.h > >> > >> ... > >> > >> > EXPORT_SYMBOL_GPL(pm_power_off); > >> > >> ^^^ (Note this and read below) > >> > >> ... > >> > >> > +EXPORT_SYMBOL(arch_get_random_seed_long); > >> > >> It can't be like this. Brief browsing of the callees shows that. > > > > Is my understanding correct that you're suggesting to make it GPL? > > > > ppc_md is exported with EXPORT_SYMBOL(), and the function is in header, > > so it's available for non-GPL code now. I don't want to change it. > > That's true, your change maintains the status quo. > > But I think we actually don't need it exported to modules, I think it's > a private detail of the RNG <-> architecture interface, not something > that modules should be calling. > > So I think it's OK to drop the EXPORT_SYMBOL, either in this patch or a > subsequent one if you don't want to rebase. OK, changed. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-23 21:45 ` [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h Yury Norov 2022-07-25 7:28 ` Andy Shevchenko @ 2022-07-25 8:34 ` Michael Ellerman 2022-07-25 12:22 ` Michael Ellerman 2022-07-25 12:22 ` Michael Ellerman 2 siblings, 1 reply; 29+ messages in thread From: Michael Ellerman @ 2022-07-25 8:34 UTC (permalink / raw) To: Yury Norov, linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell Cc: Yury Norov Yury Norov <yury.norov@gmail.com> writes: > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > circular header dependency, if generic nodemask.h includes random.h: > > In file included from include/linux/cred.h:16, > from include/linux/seq_file.h:13, > from arch/powerpc/include/asm/machdep.h:6, > from arch/powerpc/include/asm/archrandom.h:5, > from include/linux/random.h:109, > from include/linux/nodemask.h:97, > from include/linux/list_lru.h:12, > from include/linux/fs.h:13, > from include/linux/compat.h:17, > from arch/powerpc/kernel/asm-offsets.c:12: > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > 1203 | nodemask_t mems_allowed; > | ^~~~~~~~~~ > > Fix it by removing <asm/machdep.h> dependency from archrandom.h > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > --- > arch/powerpc/include/asm/archrandom.h | 9 +-------- > arch/powerpc/kernel/setup-common.c | 11 +++++++++++ > 2 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h > index 9a53e29680f4..21def59ef1a6 100644 > --- a/arch/powerpc/include/asm/archrandom.h > +++ b/arch/powerpc/include/asm/archrandom.h > @@ -4,7 +4,7 @@ > > #ifdef CONFIG_ARCH_RANDOM > > -#include <asm/machdep.h> > +bool __must_check arch_get_random_seed_long(unsigned long *v); > > static inline bool __must_check arch_get_random_long(unsigned long *v) > { > @@ -16,13 +16,6 @@ static inline bool __must_check arch_get_random_int(unsigned int *v) > return false; > } > > -static inline bool __must_check arch_get_random_seed_long(unsigned long *v) > -{ > - if (ppc_md.get_random_seed) > - return ppc_md.get_random_seed(v); > - > - return false; > -} I'd rather we didn't have to force this out of line. I think I see a different way to fix it, I'll just do some more build tests. cheers ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-25 8:34 ` Michael Ellerman @ 2022-07-25 12:22 ` Michael Ellerman 2022-07-25 16:28 ` Yury Norov 0 siblings, 1 reply; 29+ messages in thread From: Michael Ellerman @ 2022-07-25 12:22 UTC (permalink / raw) To: Yury Norov, linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell Cc: Yury Norov Michael Ellerman <mpe@ellerman.id.au> writes: > Yury Norov <yury.norov@gmail.com> writes: >> archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes >> circular header dependency, if generic nodemask.h includes random.h: >> >> In file included from include/linux/cred.h:16, >> from include/linux/seq_file.h:13, >> from arch/powerpc/include/asm/machdep.h:6, >> from arch/powerpc/include/asm/archrandom.h:5, >> from include/linux/random.h:109, >> from include/linux/nodemask.h:97, >> from include/linux/list_lru.h:12, >> from include/linux/fs.h:13, >> from include/linux/compat.h:17, >> from arch/powerpc/kernel/asm-offsets.c:12: >> include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' >> 1203 | nodemask_t mems_allowed; >> | ^~~~~~~~~~ >> >> Fix it by removing <asm/machdep.h> dependency from archrandom.h >> >> Signed-off-by: Yury Norov <yury.norov@gmail.com> >> --- >> arch/powerpc/include/asm/archrandom.h | 9 +-------- >> arch/powerpc/kernel/setup-common.c | 11 +++++++++++ >> 2 files changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h >> index 9a53e29680f4..21def59ef1a6 100644 >> --- a/arch/powerpc/include/asm/archrandom.h >> +++ b/arch/powerpc/include/asm/archrandom.h >> @@ -4,7 +4,7 @@ >> >> #ifdef CONFIG_ARCH_RANDOM >> >> -#include <asm/machdep.h> >> +bool __must_check arch_get_random_seed_long(unsigned long *v); >> >> static inline bool __must_check arch_get_random_long(unsigned long *v) >> { >> @@ -16,13 +16,6 @@ static inline bool __must_check arch_get_random_int(unsigned int *v) >> return false; >> } >> >> -static inline bool __must_check arch_get_random_seed_long(unsigned long *v) >> -{ >> - if (ppc_md.get_random_seed) >> - return ppc_md.get_random_seed(v); >> - >> - return false; >> -} > > I'd rather we didn't have to force this out of line. > > I think I see a different way to fix it, I'll just do some more build > tests. Of course my idea didn't work :} So I'll just ack your patch for now, and maybe I can get the headers cleaned up in future to allow it to be out-of-line again. cheers ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-25 12:22 ` Michael Ellerman @ 2022-07-25 16:28 ` Yury Norov 0 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-07-25 16:28 UTC (permalink / raw) To: Michael Ellerman Cc: Stephen Rothwell, Rasmus Villemoes, linux-kernel, Paul Mackerras, Andy Shevchenko, linuxppc-dev On Mon, Jul 25, 2022 at 10:22:13PM +1000, Michael Ellerman wrote: > Michael Ellerman <mpe@ellerman.id.au> writes: > > Yury Norov <yury.norov@gmail.com> writes: > >> archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > >> circular header dependency, if generic nodemask.h includes random.h: > >> > >> In file included from include/linux/cred.h:16, > >> from include/linux/seq_file.h:13, > >> from arch/powerpc/include/asm/machdep.h:6, > >> from arch/powerpc/include/asm/archrandom.h:5, > >> from include/linux/random.h:109, > >> from include/linux/nodemask.h:97, > >> from include/linux/list_lru.h:12, > >> from include/linux/fs.h:13, > >> from include/linux/compat.h:17, > >> from arch/powerpc/kernel/asm-offsets.c:12: > >> include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > >> 1203 | nodemask_t mems_allowed; > >> | ^~~~~~~~~~ > >> > >> Fix it by removing <asm/machdep.h> dependency from archrandom.h > >> > >> Signed-off-by: Yury Norov <yury.norov@gmail.com> > >> --- > >> arch/powerpc/include/asm/archrandom.h | 9 +-------- > >> arch/powerpc/kernel/setup-common.c | 11 +++++++++++ > >> 2 files changed, 12 insertions(+), 8 deletions(-) > >> > >> diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h > >> index 9a53e29680f4..21def59ef1a6 100644 > >> --- a/arch/powerpc/include/asm/archrandom.h > >> +++ b/arch/powerpc/include/asm/archrandom.h > >> @@ -4,7 +4,7 @@ > >> > >> #ifdef CONFIG_ARCH_RANDOM > >> > >> -#include <asm/machdep.h> > >> +bool __must_check arch_get_random_seed_long(unsigned long *v); > >> > >> static inline bool __must_check arch_get_random_long(unsigned long *v) > >> { > >> @@ -16,13 +16,6 @@ static inline bool __must_check arch_get_random_int(unsigned int *v) > >> return false; > >> } > >> > >> -static inline bool __must_check arch_get_random_seed_long(unsigned long *v) > >> -{ > >> - if (ppc_md.get_random_seed) > >> - return ppc_md.get_random_seed(v); > >> - > >> - return false; > >> -} > > > > I'd rather we didn't have to force this out of line. > > > > I think I see a different way to fix it, I'll just do some more build > > tests. > > Of course my idea didn't work :} > > So I'll just ack your patch for now, and maybe I can get the headers > cleaned up in future to allow it to be out-of-line again. I understand that it looks like a tradeoff - we inline a couple of small functions with the cost of uninlining an almost innocent victim. The complete solution would be probably a splitting ppc_md declaration out of asm/machdep.h. I wanted to do that, but I'm not a PPC guy, and just don't know how to split the header correctly. Anyways, thanks for the ack. Applied on bitmap-for-next. Thanks, Yury ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-25 16:28 ` Yury Norov 0 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-07-25 16:28 UTC (permalink / raw) To: Michael Ellerman Cc: linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell On Mon, Jul 25, 2022 at 10:22:13PM +1000, Michael Ellerman wrote: > Michael Ellerman <mpe@ellerman.id.au> writes: > > Yury Norov <yury.norov@gmail.com> writes: > >> archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > >> circular header dependency, if generic nodemask.h includes random.h: > >> > >> In file included from include/linux/cred.h:16, > >> from include/linux/seq_file.h:13, > >> from arch/powerpc/include/asm/machdep.h:6, > >> from arch/powerpc/include/asm/archrandom.h:5, > >> from include/linux/random.h:109, > >> from include/linux/nodemask.h:97, > >> from include/linux/list_lru.h:12, > >> from include/linux/fs.h:13, > >> from include/linux/compat.h:17, > >> from arch/powerpc/kernel/asm-offsets.c:12: > >> include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > >> 1203 | nodemask_t mems_allowed; > >> | ^~~~~~~~~~ > >> > >> Fix it by removing <asm/machdep.h> dependency from archrandom.h > >> > >> Signed-off-by: Yury Norov <yury.norov@gmail.com> > >> --- > >> arch/powerpc/include/asm/archrandom.h | 9 +-------- > >> arch/powerpc/kernel/setup-common.c | 11 +++++++++++ > >> 2 files changed, 12 insertions(+), 8 deletions(-) > >> > >> diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h > >> index 9a53e29680f4..21def59ef1a6 100644 > >> --- a/arch/powerpc/include/asm/archrandom.h > >> +++ b/arch/powerpc/include/asm/archrandom.h > >> @@ -4,7 +4,7 @@ > >> > >> #ifdef CONFIG_ARCH_RANDOM > >> > >> -#include <asm/machdep.h> > >> +bool __must_check arch_get_random_seed_long(unsigned long *v); > >> > >> static inline bool __must_check arch_get_random_long(unsigned long *v) > >> { > >> @@ -16,13 +16,6 @@ static inline bool __must_check arch_get_random_int(unsigned int *v) > >> return false; > >> } > >> > >> -static inline bool __must_check arch_get_random_seed_long(unsigned long *v) > >> -{ > >> - if (ppc_md.get_random_seed) > >> - return ppc_md.get_random_seed(v); > >> - > >> - return false; > >> -} > > > > I'd rather we didn't have to force this out of line. > > > > I think I see a different way to fix it, I'll just do some more build > > tests. > > Of course my idea didn't work :} > > So I'll just ack your patch for now, and maybe I can get the headers > cleaned up in future to allow it to be out-of-line again. I understand that it looks like a tradeoff - we inline a couple of small functions with the cost of uninlining an almost innocent victim. The complete solution would be probably a splitting ppc_md declaration out of asm/machdep.h. I wanted to do that, but I'm not a PPC guy, and just don't know how to split the header correctly. Anyways, thanks for the ack. Applied on bitmap-for-next. Thanks, Yury ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-25 16:28 ` Yury Norov @ 2022-07-26 2:46 ` Michael Ellerman -1 siblings, 0 replies; 29+ messages in thread From: Michael Ellerman @ 2022-07-26 2:46 UTC (permalink / raw) To: Yury Norov Cc: Stephen Rothwell, Rasmus Villemoes, linux-kernel, Paul Mackerras, Andy Shevchenko, linuxppc-dev Yury Norov <yury.norov@gmail.com> writes: > On Mon, Jul 25, 2022 at 10:22:13PM +1000, Michael Ellerman wrote: >> Michael Ellerman <mpe@ellerman.id.au> writes: >> > Yury Norov <yury.norov@gmail.com> writes: >> >> archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes >> >> circular header dependency, if generic nodemask.h includes random.h: >> >> >> >> In file included from include/linux/cred.h:16, >> >> from include/linux/seq_file.h:13, >> >> from arch/powerpc/include/asm/machdep.h:6, >> >> from arch/powerpc/include/asm/archrandom.h:5, >> >> from include/linux/random.h:109, >> >> from include/linux/nodemask.h:97, >> >> from include/linux/list_lru.h:12, >> >> from include/linux/fs.h:13, >> >> from include/linux/compat.h:17, >> >> from arch/powerpc/kernel/asm-offsets.c:12: >> >> include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' >> >> 1203 | nodemask_t mems_allowed; >> >> | ^~~~~~~~~~ >> >> >> >> Fix it by removing <asm/machdep.h> dependency from archrandom.h >> >> >> >> Signed-off-by: Yury Norov <yury.norov@gmail.com> >> >> --- >> >> arch/powerpc/include/asm/archrandom.h | 9 +-------- >> >> arch/powerpc/kernel/setup-common.c | 11 +++++++++++ >> >> 2 files changed, 12 insertions(+), 8 deletions(-) >> >> >> >> diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h >> >> index 9a53e29680f4..21def59ef1a6 100644 >> >> --- a/arch/powerpc/include/asm/archrandom.h >> >> +++ b/arch/powerpc/include/asm/archrandom.h >> >> @@ -4,7 +4,7 @@ >> >> >> >> #ifdef CONFIG_ARCH_RANDOM >> >> >> >> -#include <asm/machdep.h> >> >> +bool __must_check arch_get_random_seed_long(unsigned long *v); >> >> >> >> static inline bool __must_check arch_get_random_long(unsigned long *v) >> >> { >> >> @@ -16,13 +16,6 @@ static inline bool __must_check arch_get_random_int(unsigned int *v) >> >> return false; >> >> } >> >> >> >> -static inline bool __must_check arch_get_random_seed_long(unsigned long *v) >> >> -{ >> >> - if (ppc_md.get_random_seed) >> >> - return ppc_md.get_random_seed(v); >> >> - >> >> - return false; >> >> -} >> > >> > I'd rather we didn't have to force this out of line. >> > >> > I think I see a different way to fix it, I'll just do some more build >> > tests. >> >> Of course my idea didn't work :} >> >> So I'll just ack your patch for now, and maybe I can get the headers >> cleaned up in future to allow it to be out-of-line again. > > I understand that it looks like a tradeoff - we inline a couple of small > functions with the cost of uninlining an almost innocent victim. Yeah. The truth is the cost to access the RNG will far outweigh the cost of that out-of-line call, so there's no real issue. But it's also such a small function that it just cries out to be inlined :) > The complete solution would be probably a splitting ppc_md declaration > out of asm/machdep.h. I wanted to do that, but I'm not a PPC guy, and > just don't know how to split the header correctly. I managed to drop the includes of seq_file.h and dma-mapping.h, which seemed to fix the circular include problem, but there's a bit of fall out in unrelated files. I think I can get that sorted though eventually. > Anyways, thanks for the ack. Applied on bitmap-for-next. No worries. cheers ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h @ 2022-07-26 2:46 ` Michael Ellerman 0 siblings, 0 replies; 29+ messages in thread From: Michael Ellerman @ 2022-07-26 2:46 UTC (permalink / raw) To: Yury Norov Cc: linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell Yury Norov <yury.norov@gmail.com> writes: > On Mon, Jul 25, 2022 at 10:22:13PM +1000, Michael Ellerman wrote: >> Michael Ellerman <mpe@ellerman.id.au> writes: >> > Yury Norov <yury.norov@gmail.com> writes: >> >> archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes >> >> circular header dependency, if generic nodemask.h includes random.h: >> >> >> >> In file included from include/linux/cred.h:16, >> >> from include/linux/seq_file.h:13, >> >> from arch/powerpc/include/asm/machdep.h:6, >> >> from arch/powerpc/include/asm/archrandom.h:5, >> >> from include/linux/random.h:109, >> >> from include/linux/nodemask.h:97, >> >> from include/linux/list_lru.h:12, >> >> from include/linux/fs.h:13, >> >> from include/linux/compat.h:17, >> >> from arch/powerpc/kernel/asm-offsets.c:12: >> >> include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' >> >> 1203 | nodemask_t mems_allowed; >> >> | ^~~~~~~~~~ >> >> >> >> Fix it by removing <asm/machdep.h> dependency from archrandom.h >> >> >> >> Signed-off-by: Yury Norov <yury.norov@gmail.com> >> >> --- >> >> arch/powerpc/include/asm/archrandom.h | 9 +-------- >> >> arch/powerpc/kernel/setup-common.c | 11 +++++++++++ >> >> 2 files changed, 12 insertions(+), 8 deletions(-) >> >> >> >> diff --git a/arch/powerpc/include/asm/archrandom.h b/arch/powerpc/include/asm/archrandom.h >> >> index 9a53e29680f4..21def59ef1a6 100644 >> >> --- a/arch/powerpc/include/asm/archrandom.h >> >> +++ b/arch/powerpc/include/asm/archrandom.h >> >> @@ -4,7 +4,7 @@ >> >> >> >> #ifdef CONFIG_ARCH_RANDOM >> >> >> >> -#include <asm/machdep.h> >> >> +bool __must_check arch_get_random_seed_long(unsigned long *v); >> >> >> >> static inline bool __must_check arch_get_random_long(unsigned long *v) >> >> { >> >> @@ -16,13 +16,6 @@ static inline bool __must_check arch_get_random_int(unsigned int *v) >> >> return false; >> >> } >> >> >> >> -static inline bool __must_check arch_get_random_seed_long(unsigned long *v) >> >> -{ >> >> - if (ppc_md.get_random_seed) >> >> - return ppc_md.get_random_seed(v); >> >> - >> >> - return false; >> >> -} >> > >> > I'd rather we didn't have to force this out of line. >> > >> > I think I see a different way to fix it, I'll just do some more build >> > tests. >> >> Of course my idea didn't work :} >> >> So I'll just ack your patch for now, and maybe I can get the headers >> cleaned up in future to allow it to be out-of-line again. > > I understand that it looks like a tradeoff - we inline a couple of small > functions with the cost of uninlining an almost innocent victim. Yeah. The truth is the cost to access the RNG will far outweigh the cost of that out-of-line call, so there's no real issue. But it's also such a small function that it just cries out to be inlined :) > The complete solution would be probably a splitting ppc_md declaration > out of asm/machdep.h. I wanted to do that, but I'm not a PPC guy, and > just don't know how to split the header correctly. I managed to drop the includes of seq_file.h and dma-mapping.h, which seemed to fix the circular include problem, but there's a bit of fall out in unrelated files. I think I can get that sorted though eventually. > Anyways, thanks for the ack. Applied on bitmap-for-next. No worries. cheers ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h 2022-07-23 21:45 ` [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h Yury Norov 2022-07-25 7:28 ` Andy Shevchenko 2022-07-25 8:34 ` Michael Ellerman @ 2022-07-25 12:22 ` Michael Ellerman 2 siblings, 0 replies; 29+ messages in thread From: Michael Ellerman @ 2022-07-25 12:22 UTC (permalink / raw) To: Yury Norov, linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell Cc: Yury Norov Yury Norov <yury.norov@gmail.com> writes: > archrandom.h includes <asm/machdep.h> to refer ppc_md. This causes > circular header dependency, if generic nodemask.h includes random.h: > > In file included from include/linux/cred.h:16, > from include/linux/seq_file.h:13, > from arch/powerpc/include/asm/machdep.h:6, > from arch/powerpc/include/asm/archrandom.h:5, > from include/linux/random.h:109, > from include/linux/nodemask.h:97, > from include/linux/list_lru.h:12, > from include/linux/fs.h:13, > from include/linux/compat.h:17, > from arch/powerpc/kernel/asm-offsets.c:12: > include/linux/sched.h:1203:9: error: unknown type name 'nodemask_t' > 1203 | nodemask_t mems_allowed; > | ^~~~~~~~~~ > > Fix it by removing <asm/machdep.h> dependency from archrandom.h > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > --- > arch/powerpc/include/asm/archrandom.h | 9 +-------- > arch/powerpc/kernel/setup-common.c | 11 +++++++++++ > 2 files changed, 12 insertions(+), 8 deletions(-) Acked-by: Michael Ellerman <mpe@ellerman.id.au> cheers ^ permalink raw reply [flat|nested] 29+ messages in thread
* [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random() 2022-07-23 21:45 [PATCH 0/2] lib/nodemask: inline wrappers around bitmap Yury Norov 2022-07-23 21:45 ` [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h Yury Norov @ 2022-07-23 21:45 ` Yury Norov 2022-08-12 5:16 ` Aneesh Kumar K.V 1 sibling, 1 reply; 29+ messages in thread From: Yury Norov @ 2022-07-23 21:45 UTC (permalink / raw) To: linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell Cc: Yury Norov 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 <andriy.shevchenko@linux.intel.com> CC: Rasmus Villemoes <linux@rasmusvillemoes.dk> Signed-off-by: Yury Norov <yury.norov@gmail.com> --- 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 <linux/bitmap.h> #include <linux/minmax.h> #include <linux/numa.h> +#include <linux/random.h> 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_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..000000000000 --- a/lib/nodemask.c +++ /dev/null @@ -1,30 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include <linux/nodemask.h> -#include <linux/module.h> -#include <linux/random.h> - -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 ^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random() 2022-07-23 21:45 ` [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random() Yury Norov @ 2022-08-12 5:16 ` Aneesh Kumar K.V 2022-08-12 5:40 ` Yury Norov 0 siblings, 1 reply; 29+ messages in thread From: Aneesh Kumar K.V @ 2022-08-12 5:16 UTC (permalink / raw) To: Yury Norov, linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell Cc: Yury Norov Yury Norov <yury.norov@gmail.com> 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 <andriy.shevchenko@linux.intel.com> > CC: Rasmus Villemoes <linux@rasmusvillemoes.dk> > Signed-off-by: Yury Norov <yury.norov@gmail.com> > --- > 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 <linux/bitmap.h> > #include <linux/minmax.h> > #include <linux/numa.h> > +#include <linux/random.h> > > 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_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..000000000000 > --- a/lib/nodemask.c > +++ /dev/null > @@ -1,30 +0,0 @@ > -// SPDX-License-Identifier: GPL-2.0 > -#include <linux/nodemask.h> > -#include <linux/module.h> > -#include <linux/random.h> > - > -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 ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random() 2022-08-12 5:16 ` Aneesh Kumar K.V @ 2022-08-12 5:40 ` Yury Norov 0 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-08-12 5:40 UTC (permalink / raw) To: Aneesh Kumar K.V Cc: Stephen Rothwell, Rasmus Villemoes, linux-kernel, Paul Mackerras, Andy Shevchenko, linuxppc-dev On Fri, Aug 12, 2022 at 10:46:57AM +0530, Aneesh Kumar K.V wrote: > Yury Norov <yury.norov@gmail.com> 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 <andriy.shevchenko@linux.intel.com> > > CC: Rasmus Villemoes <linux@rasmusvillemoes.dk> > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > > --- > > 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 <linux/bitmap.h> > > #include <linux/minmax.h> > > #include <linux/numa.h> > > +#include <linux/random.h> > > > > 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_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..000000000000 > > --- a/lib/nodemask.c > > +++ /dev/null > > @@ -1,30 +0,0 @@ > > -// SPDX-License-Identifier: GPL-2.0 > > -#include <linux/nodemask.h> > > -#include <linux/module.h> > > -#include <linux/random.h> > > - > > -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. ^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random() @ 2022-08-12 5:40 ` Yury Norov 0 siblings, 0 replies; 29+ messages in thread From: Yury Norov @ 2022-08-12 5:40 UTC (permalink / raw) To: Aneesh Kumar K.V Cc: linux-kernel, Andy Shevchenko, Rasmus Villemoes, linuxppc-dev, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, Stephen Rothwell On Fri, Aug 12, 2022 at 10:46:57AM +0530, Aneesh Kumar K.V wrote: > Yury Norov <yury.norov@gmail.com> 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 <andriy.shevchenko@linux.intel.com> > > CC: Rasmus Villemoes <linux@rasmusvillemoes.dk> > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > > --- > > 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 <linux/bitmap.h> > > #include <linux/minmax.h> > > #include <linux/numa.h> > > +#include <linux/random.h> > > > > 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_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..000000000000 > > --- a/lib/nodemask.c > > +++ /dev/null > > @@ -1,30 +0,0 @@ > > -// SPDX-License-Identifier: GPL-2.0 > > -#include <linux/nodemask.h> > > -#include <linux/module.h> > > -#include <linux/random.h> > > - > > -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. ^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2022-08-12 5:40 UTC | newest] Thread overview: 29+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-07-23 21:45 [PATCH 0/2] lib/nodemask: inline wrappers around bitmap Yury Norov 2022-07-23 21:45 ` [PATCH 1/2] powerpc: drop dependency on <asm/machdep.h> in archrandom.h Yury Norov 2022-07-25 7:28 ` Andy Shevchenko 2022-07-25 7:28 ` Andy Shevchenko 2022-07-25 16:17 ` Yury Norov 2022-07-25 16:17 ` Yury Norov 2022-07-25 21:39 ` Andy Shevchenko 2022-07-25 21:39 ` Andy Shevchenko 2022-07-25 23:32 ` Yury Norov 2022-07-25 23:32 ` Yury Norov 2022-07-26 6:13 ` Andy Shevchenko 2022-07-26 6:13 ` Andy Shevchenko 2022-07-26 6:15 ` Andy Shevchenko 2022-07-26 6:15 ` Andy Shevchenko 2022-07-26 6:57 ` Michael Ellerman 2022-07-26 6:57 ` Michael Ellerman 2022-07-26 15:19 ` Yury Norov 2022-07-26 15:19 ` Yury Norov 2022-07-25 8:34 ` Michael Ellerman 2022-07-25 12:22 ` Michael Ellerman 2022-07-25 16:28 ` Yury Norov 2022-07-25 16:28 ` Yury Norov 2022-07-26 2:46 ` Michael Ellerman 2022-07-26 2:46 ` Michael Ellerman 2022-07-25 12:22 ` Michael Ellerman 2022-07-23 21:45 ` [RESEND PATCH 2/2] lib/nodemask: inline next_node_in() and node_random() Yury Norov 2022-08-12 5:16 ` Aneesh Kumar K.V 2022-08-12 5:40 ` Yury Norov 2022-08-12 5:40 ` Yury Norov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.