Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 09, 2019 at 11:56:03AM -0400, Mathieu Desnoyers wrote: > - On Apr 9, 2019, at 11:40 AM, Joel Fernandes, Google > j...@joelfernandes.org wrote: > > > On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote: > >> - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote: > >> > >> > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote: > >> >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com wrote: > >> >> > >> >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote: > >> >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com > >> >> >> wrote: > >> >> >> > >> >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +0000, Joel Fernandes wrote: > >> >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote: > >> >> >> >> > > >> >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google > >> >> >> >> > j...@joelfernandes.org > >> >> >> >> > wrote: > >> >> >> >> > > >> >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers > >> >> >> >> > > wrote: > >> >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck > >> >> >> >> > >> paul...@linux.ibm.com wrote: > >> >> >> >> > >> > >> >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney > >> >> >> >> > >> > wrote: > >> >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes > >> >> >> >> > >> >> wrote: > >> >> >> >> > >> > > >> >> >> >> > >> > [ . . . ] > >> >> >> >> > >> > > >> >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644 > >> >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h > >> >> >> >> > >> >> > > @@ -338,6 +338,10 @@ > >> >> >> >> > >> >> > > KEEP(*(__tracepoints_ptrs)) /* > >> >> >> >> > >> >> > > Tracepoints: pointer array */ \ > >> >> >> >> > >> >> > > __stop___tracepoints_ptrs = .; > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > *(__tracepoints_strings)/* Tracepoints: > >> >> >> >> > >> >> > > strings */ \ > >> >> >> >> > >> >> > > + . = ALIGN(8); > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > + __start___srcu_struct = .; > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > + *(___srcu_struct_ptrs) > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > + __end___srcu_struct = .; > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > } > >> >> >> >> > >> >> > > \ > >> >> >> >> > >> >> > > >> >> >> >> > >> >> > This vmlinux linker modification is not needed. I > >> >> >> &g
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Mon, Apr 08, 2019 at 01:24:47PM -0400, Mathieu Desnoyers wrote: > - On Apr 8, 2019, at 11:46 AM, paulmck paul...@linux.ibm.com wrote: > > > On Mon, Apr 08, 2019 at 10:49:32AM -0400, Mathieu Desnoyers wrote: > >> - On Apr 8, 2019, at 10:22 AM, paulmck paul...@linux.ibm.com wrote: > >> > >> > On Mon, Apr 08, 2019 at 09:05:34AM -0400, Mathieu Desnoyers wrote: > >> >> - On Apr 7, 2019, at 10:27 PM, paulmck paul...@linux.ibm.com wrote: > >> >> > >> >> > On Sun, Apr 07, 2019 at 09:07:18PM +, Joel Fernandes wrote: > >> >> >> On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote: > >> >> >> > > >> >> >> > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google > >> >> >> > j...@joelfernandes.org > >> >> >> > wrote: > >> >> >> > > >> >> >> > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers > >> >> >> > > wrote: > >> >> >> > >> - On Apr 7, 2019, at 9:59 AM, paulmck paul...@linux.ibm.com > >> >> >> > >> wrote: > >> >> >> > >> > >> >> >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney > >> >> >> > >> > wrote: > >> >> >> > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes > >> >> >> > >> >> wrote: > >> >> >> > >> > > >> >> >> > >> > [ . . . ] > >> >> >> > >> > > >> >> >> > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h > >> >> >> > >> >> > > b/include/asm-generic/vmlinux.lds.h > >> >> >> > >> >> > > index f8f6f04c4453..c2d919a1566e 100644 > >> >> >> > >> >> > > --- a/include/asm-generic/vmlinux.lds.h > >> >> >> > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h > >> >> >> > >> >> > > @@ -338,6 +338,10 @@ > >> >> >> > >> >> > > KEEP(*(__tracepoints_ptrs)) /* > >> >> >> > >> >> > > Tracepoints: pointer array */ \ > >> >> >> > >> >> > > __stop___tracepoints_ptrs = .; > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > *(__tracepoints_strings)/* Tracepoints: > >> >> >> > >> >> > > strings */ \ > >> >> >> > >> >> > > +. = ALIGN(8); > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > +__start___srcu_struct = .; > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > +*(___srcu_struct_ptrs) > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > +__end___srcu_struct = .; > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > } > >> >> >> > >> >> > > \ > >> >> >> > >> >> > > >> >> >> > >> >> > This vmlinux linker modification is not needed. I tested > >> >> >> > >> >> > without it and srcu > >> >> >> > >> >> > torture works fine with rcutorture built as a module. > >> >> >> > >> >> > Putting further prints > >> >> >> > >> >> > in kernel/module.c verified that the kernel is able to > >> >> >> > >> >> > find the srcu structs > >> >> >> > >> >> > just fine. You could squash the below patch into this one > >> >> >> > >> >> > or apply it on top >
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Sun, Apr 07, 2019 at 10:05:14AM -0700, Paul E. McKenney wrote: > On Sun, Apr 07, 2019 at 03:46:13PM +0000, Joel Fernandes wrote: > > On Sun, Apr 07, 2019 at 06:59:37AM -0700, Paul E. McKenney wrote: > > > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote: > > > > On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote: > > > > > > [ . . . ] > > > > > > > > > diff --git a/include/asm-generic/vmlinux.lds.h > > > > > > b/include/asm-generic/vmlinux.lds.h > > > > > > index f8f6f04c4453..c2d919a1566e 100644 > > > > > > --- a/include/asm-generic/vmlinux.lds.h > > > > > > +++ b/include/asm-generic/vmlinux.lds.h > > > > > > @@ -338,6 +338,10 @@ > > > > > > KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer > > > > > > array */ \ > > > > > > __stop___tracepoints_ptrs = .; > > > > > > \ > > > > > > *(__tracepoints_strings)/* Tracepoints: strings */ > > > > > > \ > > > > > > + . = ALIGN(8); > > > > > > \ > > > > > > + __start___srcu_struct = .; > > > > > > \ > > > > > > + *(___srcu_struct_ptrs) > > > > > > \ > > > > > > + __end___srcu_struct = .; > > > > > > \ > > > > > > } > > > > > > \ > > > > > > > > > > This vmlinux linker modification is not needed. I tested without it > > > > > and srcu > > > > > torture works fine with rcutorture built as a module. Putting further > > > > > prints > > > > > in kernel/module.c verified that the kernel is able to find the srcu > > > > > structs > > > > > just fine. You could squash the below patch into this one or apply it > > > > > on top > > > > > of the dev branch. > > > > > > > > Good point, given that otherwise FORTRAN named common blocks would not > > > > work. > > > > > > > > But isn't one advantage of leaving that stuff in the RO_DATA_SECTION() > > > > macro that it can be mapped read-only? Or am I suffering from excessive > > > > optimism? > > > > > > And to answer the other question, in the case where I am suffering from > > > excessive optimism, it should be a separate commit. Please see below > > > for the updated original commit thus far. > > > > Actually the vmlinux.lds.h file is unused for module building. For ex, if > > you > > delete include/asm-generic/vmlinux.lds.h , then you can still build > > rcutorture.ko. Did I miss something obvious? In that case the vmlinux.lds.h > > are not needed since the __section annotations automatically place the srcu > > structs in a separate section. > > Hard to argue given that I just deleted include/asm-generic/vmlinux.lds.h, > touched kernel/rcu/rcutorture.c, and rebuilt the corresponding .ko > without errors. ;-) > > Hmmm... Is there some way to place a section into a read-only page, > for example, tagged onto the text section for that module? That would > get rid of a class of bugs, if nothing else. Strictly speaking, the array of pointers in the new srcu section are fixed up at runtime because the srcu_struct(s) they point to can be loaded at a dynamic location in memory. The srcu_struct(s) are themselves in the .bss section of the module and their locations depend on where the .bss section of the module is loaded in memory at load time. I agree that after such relocation fixups are done, there is no reason to keep the array-of-pointers section readable but unfortunately I couldn't figure a way out to make them read-only post the relocations. I copied Jessica Yu who maintains module loading for any input. Jessica, as a summary, we are trying to create a custom ELF section of srcu_struct pointers in kernels modules, and then make the module loader do SRCU initialization from structs pointed to by this section. The srcu_struct themselves are defined on the .bss section. Is there any way we can make this pointer array section read-only *after* the relocation fixups of the array are completed? > > Let me know if you would like me to send a patch separately, or if the > > appended patch for the same in my previous email suffices. > > Please do resend as a formal patch with the above in the commit log. > I doubt that I am the only one needing a bit of module-build education! > And thank you for providing that education, by the way! Sounds great, I will go ahead and send out a patch in the morning for this part. > > > And may I have your Tested-by? > > > > Absolutely, please do and thanks! > > Done, and thank you for giving it a go! You're very welcome. thanks, - Joel ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Sun, Apr 07, 2019 at 04:41:36PM -0400, Mathieu Desnoyers wrote: > > - On Apr 7, 2019, at 3:32 PM, Joel Fernandes, Google > j...@joelfernandes.org wrote: > > > On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers wrote: > >> - On Apr 7, 2019, at 9:59 AM, paulmck paul...@linux.ibm.com wrote: > >> > >> > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote: > >> >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote: > >> > > >> > [ . . . ] > >> > > >> >> > > diff --git a/include/asm-generic/vmlinux.lds.h > >> >> > > b/include/asm-generic/vmlinux.lds.h > >> >> > > index f8f6f04c4453..c2d919a1566e 100644 > >> >> > > --- a/include/asm-generic/vmlinux.lds.h > >> >> > > +++ b/include/asm-generic/vmlinux.lds.h > >> >> > > @@ -338,6 +338,10 @@ > >> >> > > KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer > >> >> > > array */ \ > >> >> > > __stop___tracepoints_ptrs = .; > >> >> > > \ > >> >> > > *(__tracepoints_strings)/* Tracepoints: strings */ > >> >> > > \ > >> >> > > + . = ALIGN(8); > >> >> > > \ > >> >> > > + __start___srcu_struct = .; > >> >> > > \ > >> >> > > + *(___srcu_struct_ptrs) > >> >> > > \ > >> >> > > + __end___srcu_struct = .; > >> >> > > \ > >> >> > > } > >> >> > > \ > >> >> > > >> >> > This vmlinux linker modification is not needed. I tested without it > >> >> > and srcu > >> >> > torture works fine with rcutorture built as a module. Putting further > >> >> > prints > >> >> > in kernel/module.c verified that the kernel is able to find the srcu > >> >> > structs > >> >> > just fine. You could squash the below patch into this one or apply it > >> >> > on top > >> >> > of the dev branch. > >> >> > >> >> Good point, given that otherwise FORTRAN named common blocks would not > >> >> work. > >> >> > >> >> But isn't one advantage of leaving that stuff in the RO_DATA_SECTION() > >> >> macro that it can be mapped read-only? Or am I suffering from excessive > >> >> optimism? > >> > > >> > And to answer the other question, in the case where I am suffering from > >> > excessive optimism, it should be a separate commit. Please see below > >> > for the updated original commit thus far. > >> > > >> > And may I have your Tested-by? > >> > >> Just to confirm: does the cleanup performed in the modules going > >> notifier end up acting as a barrier first before freeing the memory ? > >> If not, is it explicitly stated that a barrier must be issued before > >> module unload ? > >> > > > > You mean rcu_barrier? It is mentioned in the documentation that this is the > > responsibility of the module writer to prevent delays for all modules. > > It's a srcu barrier yes. Considering it would be a barrier specific to the > srcu domain within that module, I don't see how it would cause delays for > "all" modules if we implicitly issue the barrier on module unload. What > am I missing ? Yes you are right. I thought of this after I just sent my email. I think it makes sense for srcu case to do and could avoid a class of bugs. thanks! > > Thanks, > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Sun, Apr 07, 2019 at 03:26:16PM -0400, Mathieu Desnoyers wrote: > - On Apr 7, 2019, at 9:59 AM, paulmck paul...@linux.ibm.com wrote: > > > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote: > >> On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote: > > > > [ . . . ] > > > >> > > diff --git a/include/asm-generic/vmlinux.lds.h > >> > > b/include/asm-generic/vmlinux.lds.h > >> > > index f8f6f04c4453..c2d919a1566e 100644 > >> > > --- a/include/asm-generic/vmlinux.lds.h > >> > > +++ b/include/asm-generic/vmlinux.lds.h > >> > > @@ -338,6 +338,10 @@ > >> > >KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer > >> > > array */ \ > >> > >__stop___tracepoints_ptrs = .; > >> > > \ > >> > >*(__tracepoints_strings)/* Tracepoints: strings */ > >> > > \ > >> > > + . = ALIGN(8); > >> > > \ > >> > > + __start___srcu_struct = .; > >> > > \ > >> > > + *(___srcu_struct_ptrs) > >> > > \ > >> > > + __end___srcu_struct = .; > >> > > \ > >> > >} > >> > > \ > >> > > >> > This vmlinux linker modification is not needed. I tested without it and > >> > srcu > >> > torture works fine with rcutorture built as a module. Putting further > >> > prints > >> > in kernel/module.c verified that the kernel is able to find the srcu > >> > structs > >> > just fine. You could squash the below patch into this one or apply it on > >> > top > >> > of the dev branch. > >> > >> Good point, given that otherwise FORTRAN named common blocks would not > >> work. > >> > >> But isn't one advantage of leaving that stuff in the RO_DATA_SECTION() > >> macro that it can be mapped read-only? Or am I suffering from excessive > >> optimism? > > > > And to answer the other question, in the case where I am suffering from > > excessive optimism, it should be a separate commit. Please see below > > for the updated original commit thus far. > > > > And may I have your Tested-by? > > Just to confirm: does the cleanup performed in the modules going > notifier end up acting as a barrier first before freeing the memory ? > If not, is it explicitly stated that a barrier must be issued before > module unload ? > You mean rcu_barrier? It is mentioned in the documentation that this is the responsibility of the module writer to prevent delays for all modules. thanks. > Thanks, > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Sun, Apr 07, 2019 at 06:59:37AM -0700, Paul E. McKenney wrote: > On Sun, Apr 07, 2019 at 06:39:41AM -0700, Paul E. McKenney wrote: > > On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote: > > [ . . . ] > > > > > diff --git a/include/asm-generic/vmlinux.lds.h > > > > b/include/asm-generic/vmlinux.lds.h > > > > index f8f6f04c4453..c2d919a1566e 100644 > > > > --- a/include/asm-generic/vmlinux.lds.h > > > > +++ b/include/asm-generic/vmlinux.lds.h > > > > @@ -338,6 +338,10 @@ > > > > KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer > > > > array */ \ > > > > __stop___tracepoints_ptrs = .; > > > > \ > > > > *(__tracepoints_strings)/* Tracepoints: strings */ > > > > \ > > > > + . = ALIGN(8); > > > > \ > > > > + __start___srcu_struct = .; > > > > \ > > > > + *(___srcu_struct_ptrs) > > > > \ > > > > + __end___srcu_struct = .; > > > > \ > > > > } > > > > \ > > > > > > This vmlinux linker modification is not needed. I tested without it and > > > srcu > > > torture works fine with rcutorture built as a module. Putting further > > > prints > > > in kernel/module.c verified that the kernel is able to find the srcu > > > structs > > > just fine. You could squash the below patch into this one or apply it on > > > top > > > of the dev branch. > > > > Good point, given that otherwise FORTRAN named common blocks would not > > work. > > > > But isn't one advantage of leaving that stuff in the RO_DATA_SECTION() > > macro that it can be mapped read-only? Or am I suffering from excessive > > optimism? > > And to answer the other question, in the case where I am suffering from > excessive optimism, it should be a separate commit. Please see below > for the updated original commit thus far. > Actually the vmlinux.lds.h file is unused for module building. For ex, if you delete include/asm-generic/vmlinux.lds.h , then you can still build rcutorture.ko. Did I miss something obvious? In that case the vmlinux.lds.h are not needed since the __section annotations automatically place the srcu structs in a separate section. Let me know if you would like me to send a patch separately, or if the appended patch for the same in my previous email suffices. > And may I have your Tested-by? Absolutely, please do and thanks! - Joel > Thanx, Paul > > > > commit a365bb5f6eafb220a1448674054b05c250829313 > Author: Paul E. McKenney > Date: Fri Apr 5 16:15:00 2019 -0700 > > srcu: Allocate per-CPU data for DEFINE_SRCU() in modules > > Adding DEFINE_SRCU() or DEFINE_STATIC_SRCU() to a loadable module requires > that the size of the reserved region be increased, which is not something > we want to be doing all that often. One approach would be to require > that loadable modules define an srcu_struct and invoke init_srcu_struct() > from their module_init function and cleanup_srcu_struct() from their > module_exit function. However, this is more than a bit user unfriendly. > > This commit therefore creates an ___srcu_struct_ptrs linker section, > and pointers to srcu_struct structures created by DEFINE_SRCU() and > DEFINE_STATIC_SRCU() within a module are placed into that module's > ___srcu_struct_ptrs section. The required init_srcu_struct() and > cleanup_srcu_struct() functions are then automatically invoked as needed > when that module is loaded and unloaded, thus allowing modules to continue > to use DEFINE_SRCU() and DEFINE_STATIC_SRCU() while avoiding the need > to increase the size of the reserved region. > > Many of the algorithms and some of the code was cheerfully cherry-picked > from other code making use of linker sections, perhaps most notably from > tracepoints. All bugs are nevertheless the sole property of the author. > > Suggested-by: Mathieu Desnoyers > [ paulmck: Use __section() and use "default" in srcu_module_notify()'s > "switch" statement a
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
be to require
> that loadable modules define an srcu_struct and invoke init_srcu_struct()
> from their module_init function and cleanup_srcu_struct() from their
> module_exit function. However, this is more than a bit user unfriendly.
>
> This commit therefore creates an ___srcu_struct_ptrs linker section,
> and pointers to srcu_struct structures created by DEFINE_SRCU() and
> DEFINE_STATIC_SRCU() within a module are placed into that module's
> ___srcu_struct_ptrs section. The required init_srcu_struct() and
> cleanup_srcu_struct() functions are then automatically invoked as needed
> when that module is loaded and unloaded, thus allowing modules to continue
> to use DEFINE_SRCU() and DEFINE_STATIC_SRCU() while avoiding the need
> to increase the size of the reserved region.
>
> Many of the algorithms and some of the code was cheerfully cherry-picked
> from other code making use of linker sections, perhaps most notably from
> tracepoints. All bugs are nevertheless the sole property of the author.
>
> Suggested-by: Mathieu Desnoyers
> Signed-off-by: Paul E. McKenney
>
> diff --git a/include/asm-generic/vmlinux.lds.h
> b/include/asm-generic/vmlinux.lds.h
> index f8f6f04c4453..c2d919a1566e 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -338,6 +338,10 @@
> KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \
> __stop___tracepoints_ptrs = .; \
> *(__tracepoints_strings)/* Tracepoints: strings */ \
> + . = ALIGN(8); \
> + __start___srcu_struct = .; \
> + *(___srcu_struct_ptrs) \
> + __end___srcu_struct = .;\
> } \
This vmlinux linker modification is not needed. I tested without it and srcu
torture works fine with rcutorture built as a module. Putting further prints
in kernel/module.c verified that the kernel is able to find the srcu structs
just fine. You could squash the below patch into this one or apply it on top
of the dev branch.
Thanks!
---8<---
>From 369ad090f706ce8e1facdd18eb10828b5f7e2b72 Mon Sep 17 00:00:00 2001
From: "Joel Fernandes (Google)"
Date: Sat, 6 Apr 2019 18:57:17 -0400
Subject: [PATCH] srcu: Remove unused vmlinux srcu linker entries
The SRCU for modules optimization introduced vmlinux linker entries
which is unused since it applies only to the built-in vmlinux. So remove
it to prevent any space usage due to the 8 byte alignment.
Tested with SRCU torture_type and rcutorture.
Cc: kernel-t...@android.com
Cc: paul...@linux.vnet.ibm.com
Signed-off-by: Joel Fernandes (Google)
---
include/asm-generic/vmlinux.lds.h | 4
1 file changed, 4 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index c2d919a1566e..f8f6f04c4453 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -338,10 +338,6 @@
KEEP(*(__tracepoints_ptrs)) /* Tracepoints: pointer array */ \
__stop___tracepoints_ptrs = .; \
*(__tracepoints_strings)/* Tracepoints: strings */ \
- . = ALIGN(8); \
- __start___srcu_struct = .; \
- *(___srcu_struct_ptrs) \
- __end___srcu_struct = .;\
} \
\
.rodata1 : AT(ADDR(.rodata1) - LOAD_OFFSET) { \
--
2.21.0.392.gf8f6787159e-goog
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Fri, Apr 05, 2019 at 04:28:35PM -0700, Paul E. McKenney wrote:
> On Wed, Apr 03, 2019 at 09:20:39AM -0700, Paul E. McKenney wrote:
> > On Wed, Apr 03, 2019 at 10:27:42AM -0400, Mathieu Desnoyers wrote:
> > > - On Apr 3, 2019, at 9:32 AM, paulmck paul...@linux.ibm.com wrote:
> > >
> > > > On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> > > >> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com wrote:
> > > >>
> > > >> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> > > >> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com
> > > >> >> wrote:
> > > >> >>
> > > >> >> > Hello!
> > > >> >> >
> > > >> >> > This series prohibits use of DEFINE_SRCU() and
> > > >> >> > DEFINE_STATIC_SRCU()
> > > >> >> > by loadable modules. The reason for this prohibition is the fact
> > > >> >> > that using these two macros within modules requires that the size
> > > >> >> > of
> > > >> >> > the reserved region be increased, which is not something we want
> > > >> >> > to
> > > >> >> > be doing all that often. Instead, loadable modules should define
> > > >> >> > an
> > > >> >> > srcu_struct and invoke init_srcu_struct() from their module_init
> > > >> >> > function
> > > >> >> > and cleanup_srcu_struct() from their module_exit function. Note
> > > >> >> > that
> > > >> >> > modules using call_srcu() will also need to invoke srcu_barrier()
> > > >> >> > from
> > > >> >> > their module_exit function.
> > > >> >>
> > > >> >> This arbitrary API limitation seems weird.
> > > >> >>
> > > >> >> Isn't there a way to allow modules to use DEFINE_SRCU and
> > > >> >> DEFINE_STATIC_SRCU
> > > >> >> while implementing them with dynamic allocation under the hood ?
> > > >> >
> > > >> > Although call_srcu() already has initialization hooks, some would
> > > >> > also be required in srcu_read_lock(), and I am concerned about adding
> > > >> > memory allocation at that point, especially given the possibility
> > > >> > of memory-allocation failure. And the possibility that the first
> > > >> > srcu_read_lock() happens in an interrupt handler or similar.
> > > >> >
> > > >> > Or am I missing a trick here?
> > > >>
> > > >> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> > > >> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> > > >> would additionally lookup that section on module load, and deal with
> > > >> those statically defined SRCU entries as if they were dynamically
> > > >> allocated ones. It would of course cleanup those resources on module
> > > >> unload.
> > > >>
> > > >> Am I missing some subtlety there ?
> > > >
> > > > If I understand you correctly, that is actually what is already done.
> > > > The
> > > > size of this dedicated section is currently set by
> > > > PERCPU_MODULE_RESERVE,
> > > > and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring
> > > > that
> > > > this to be increased frequently. That led to a request that something
> > > > be done, in turn leading to this patch series.
> > >
> > > I think we are not expressing quite the same idea.
> > >
> > > AFAIU, yours is to have DEFINE*_SRCU directly define per-cpu data within
> > > modules,
> > > which ends up using percpu module reserved memory.
> > >
> > > My idea is to make DEFINE*_SRCU have a different behavior under #ifdef
> > > MODULE.
> > > It could emit a _global variable_ (_not_ per-cpu) within a new section.
> > > That
> > > section would then be used by module init/exit code to figure out what
> > > "srcu
> > > descriptors" are present in the modules. It would therefore rely on
> > > dynamic
> > > allocation for those, therefore removing the need to involve the percpu
> > > module
> > > reserved pool at all.
> > >
> > > >
> > > > I don't see a way around this short of changing module loading to do
> > > > alloc_percpu() and then updating the relocation based on this result.
> > > > Which would admittedly be far more convenient. I was assuming that
> > > > this would be difficult due to varying CPU offsets or the like.
> > > >
> > > > But if it can be done reasonably, it would be quite a bit nicer than
> > > > forcing dynamic allocation in cases where it is not otherwise needed.
> > >
> > > Hopefully my explanation above helps clear out what I have in mind.
> > >
> > > You can find similar tricks performed by include/linux/tracepoint.h:
> > >
> > > #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> > > static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> > > {
> > > return offset_to_ptr(p);
> > > }
> > >
> > > #define __TRACEPOINT_ENTRY(name)\
> > > asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
> > > " .balign 4 \n" \
> > > " .long __tracepoint_" #name " - . \n" \
> > > " .previous
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Wed, Apr 03, 2019 at 09:20:39AM -0700, Paul E. McKenney wrote:
> On Wed, Apr 03, 2019 at 10:27:42AM -0400, Mathieu Desnoyers wrote:
> > - On Apr 3, 2019, at 9:32 AM, paulmck paul...@linux.ibm.com wrote:
> >
> > > On Tue, Apr 02, 2019 at 11:34:07AM -0400, Mathieu Desnoyers wrote:
> > >> - On Apr 2, 2019, at 11:23 AM, paulmck paul...@linux.ibm.com wrote:
> > >>
> > >> > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote:
> > >> >> - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com
> > >> >> wrote:
> > >> >>
> > >> >> > Hello!
> > >> >> >
> > >> >> > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU()
> > >> >> > by loadable modules. The reason for this prohibition is the fact
> > >> >> > that using these two macros within modules requires that the size of
> > >> >> > the reserved region be increased, which is not something we want to
> > >> >> > be doing all that often. Instead, loadable modules should define an
> > >> >> > srcu_struct and invoke init_srcu_struct() from their module_init
> > >> >> > function
> > >> >> > and cleanup_srcu_struct() from their module_exit function. Note
> > >> >> > that
> > >> >> > modules using call_srcu() will also need to invoke srcu_barrier()
> > >> >> > from
> > >> >> > their module_exit function.
> > >> >>
> > >> >> This arbitrary API limitation seems weird.
> > >> >>
> > >> >> Isn't there a way to allow modules to use DEFINE_SRCU and
> > >> >> DEFINE_STATIC_SRCU
> > >> >> while implementing them with dynamic allocation under the hood ?
> > >> >
> > >> > Although call_srcu() already has initialization hooks, some would
> > >> > also be required in srcu_read_lock(), and I am concerned about adding
> > >> > memory allocation at that point, especially given the possibility
> > >> > of memory-allocation failure. And the possibility that the first
> > >> > srcu_read_lock() happens in an interrupt handler or similar.
> > >> >
> > >> > Or am I missing a trick here?
> > >>
> > >> I was more thinking that under #ifdef MODULE, both DEFINE_SRCU and
> > >> DEFINE_STATIC_SRCU could append data in a dedicated section. module.c
> > >> would additionally lookup that section on module load, and deal with
> > >> those statically defined SRCU entries as if they were dynamically
> > >> allocated ones. It would of course cleanup those resources on module
> > >> unload.
> > >>
> > >> Am I missing some subtlety there ?
> > >
> > > If I understand you correctly, that is actually what is already done. The
> > > size of this dedicated section is currently set by PERCPU_MODULE_RESERVE,
> > > and the additions of DEFINE{_STATIC}_SRCU() in modules was requiring that
> > > this to be increased frequently. That led to a request that something
> > > be done, in turn leading to this patch series.
> >
> > I think we are not expressing quite the same idea.
> >
> > AFAIU, yours is to have DEFINE*_SRCU directly define per-cpu data within
> > modules,
> > which ends up using percpu module reserved memory.
> >
> > My idea is to make DEFINE*_SRCU have a different behavior under #ifdef
> > MODULE.
> > It could emit a _global variable_ (_not_ per-cpu) within a new section. That
> > section would then be used by module init/exit code to figure out what "srcu
> > descriptors" are present in the modules. It would therefore rely on dynamic
> > allocation for those, therefore removing the need to involve the percpu
> > module
> > reserved pool at all.
> >
> > >
> > > I don't see a way around this short of changing module loading to do
> > > alloc_percpu() and then updating the relocation based on this result.
> > > Which would admittedly be far more convenient. I was assuming that
> > > this would be difficult due to varying CPU offsets or the like.
> > >
> > > But if it can be done reasonably, it would be quite a bit nicer than
> > > forcing dynamic allocation in cases where it is not otherwise needed.
> >
> > Hopefully my explanation above helps clear out what I have in mind.
> >
> > You can find similar tricks performed by include/linux/tracepoint.h:
> >
> > #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> > static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> > {
> > return offset_to_ptr(p);
> > }
> >
> > #define __TRACEPOINT_ENTRY(name)\
> > asm(" .section \"__tracepoints_ptrs\", \"a\" \n" \
> > " .balign 4 \n" \
> > " .long __tracepoint_" #name " - . \n" \
> > " .previous \n")
> > #else
> > static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
> > {
> > return *p;
> > }
> >
> > #define __TRACEPOINT_ENTRY(name) \
> > static tracepoint_ptr_t __tracepoint_ptr_##name __used \
> > __attribute__((section("_
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 02, 2019 at 08:23:34AM -0700, Paul E. McKenney wrote: > On Tue, Apr 02, 2019 at 11:14:40AM -0400, Mathieu Desnoyers wrote: > > - On Apr 2, 2019, at 10:28 AM, paulmck paul...@linux.ibm.com wrote: > > > > > Hello! > > > > > > This series prohibits use of DEFINE_SRCU() and DEFINE_STATIC_SRCU() > > > by loadable modules. The reason for this prohibition is the fact > > > that using these two macros within modules requires that the size of > > > the reserved region be increased, which is not something we want to > > > be doing all that often. Instead, loadable modules should define an > > > srcu_struct and invoke init_srcu_struct() from their module_init function > > > and cleanup_srcu_struct() from their module_exit function. Note that > > > modules using call_srcu() will also need to invoke srcu_barrier() from > > > their module_exit function. > > > > This arbitrary API limitation seems weird. > > > > Isn't there a way to allow modules to use DEFINE_SRCU and DEFINE_STATIC_SRCU > > while implementing them with dynamic allocation under the hood ? > > Although call_srcu() already has initialization hooks, some would > also be required in srcu_read_lock(), and I am concerned about adding > memory allocation at that point, especially given the possibility > of memory-allocation failure. And the possibility that the first > srcu_read_lock() happens in an interrupt handler or similar. > > Or am I missing a trick here? Hi Paul, Which 'reserved region' are you referring to? Isn't this region also used for non-module cases in which case the same problem applies to non-modules? thanks! - Joel ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
