Re: Stack-frame warnings in display_mode_vba_32.c
On Sat, Jul 30, 2022 at 02:06:10AM -0700, Guenter Roeck wrote: > On 7/29/22 22:12, Paul E. McKenney wrote: > > On Fri, Jul 29, 2022 at 11:41:55PM -0300, André Almeida wrote: > > > Hi Paul, > > > > > > Às 23:25 de 29/07/22, Paul E. McKenney escreveu: > > > > Hello! > > > > > > > > I am seeing the following in allmodconfig builds of recent -next on x86: > > > > > > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c: > > > > In function > > > > ‘DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation’: > > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1659:1: > > > > error: the frame size of 2144 bytes is larger than 2048 bytes > > > > [-Werror=frame-larger-than=] > > > > 1659 | } > > > >| ^ > > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c: > > > > In function ‘dml32_ModeSupportAndSystemConfigurationFull’: > > > > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:3799:1: > > > > error: the frame size of 2480 bytes is larger than 2048 bytes > > > > [-Werror=frame-larger-than=] > > > > 3799 | } // ModeSupportAndSystemConfigurationFull > > > >| ^ > > > > > > I think they are fixed at amd-staging-drm-next: > > > > > > git log --oneline amd/amd-staging-drm-next > > > drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c > > > 953daa61981b drm/amd/display: Reduce stack size in the mode support > > > function > > > 361e705e712d drm/amd/display: reduce stack for > > > dml32_CalculatePrefetchSchedule > > > f2dbf5a4dd1e drm/amd/display: reduce stack for > > > dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport > > > a0a68cda2ef8 drm/amd/display: reduce stack for > > > dml32_CalculateVMRowAndSwath > > > ca6730ca0f01 drm/amd/display: reduce stack for > > > dml32_CalculateSwathAndDETConfiguration > > > 593eef8c1a5e drm/amd/display: reduce stack size in dcn32 dml (v2) > > > > > > https://gitlab.freedesktop.org/agd5f/linux/-/commits/amd-staging-drm-next/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c > > > > Very good, thank you! I will test again on the next -next. > > > > Did you try next-20220728 ? > > groeck@server:~/src/linux-next$ git describe > next-20220728 > groeck@server:~/src/linux-next$ git log --oneline > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c > 1b54a0121dba drm/amd/display: Reduce stack size in the mode support function > 86e4863e67a9 drm/amd/display: reduce stack for dml32_CalculatePrefetchSchedule > 3c3abac60117 drm/amd/display: reduce stack for > dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport > c3b3f9ba25e6 drm/amd/display: reduce stack for dml32_CalculateVMRowAndSwath > bac4b41d917a drm/amd/display: reduce stack for > dml32_CalculateSwathAndDETConfiguration > 7acc487ab57e drm/amd/display: reduce stack size in dcn32 dml (v2) Indeed, next-20220728 does avoid the problem, thank you! Thanx, Paul
Re: Stack-frame warnings in display_mode_vba_32.c
On Fri, Jul 29, 2022 at 11:41:55PM -0300, André Almeida wrote:
> Hi Paul,
>
> Às 23:25 de 29/07/22, Paul E. McKenney escreveu:
> > Hello!
> >
> > I am seeing the following in allmodconfig builds of recent -next on x86:
> >
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:
> > In function
> > ‘DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation’:
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1659:1:
> > error: the frame size of 2144 bytes is larger than 2048 bytes
> > [-Werror=frame-larger-than=]
> > 1659 | }
> > | ^
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:
> > In function ‘dml32_ModeSupportAndSystemConfigurationFull’:
> > drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:3799:1:
> > error: the frame size of 2480 bytes is larger than 2048 bytes
> > [-Werror=frame-larger-than=]
> > 3799 | } // ModeSupportAndSystemConfigurationFull
> > | ^
>
> I think they are fixed at amd-staging-drm-next:
>
> git log --oneline amd/amd-staging-drm-next
> drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c
> 953daa61981b drm/amd/display: Reduce stack size in the mode support function
> 361e705e712d drm/amd/display: reduce stack for
> dml32_CalculatePrefetchSchedule
> f2dbf5a4dd1e drm/amd/display: reduce stack for
> dml32_CalculateWatermarksMALLUseAndDRAMSpeedChangeSupport
> a0a68cda2ef8 drm/amd/display: reduce stack for dml32_CalculateVMRowAndSwath
> ca6730ca0f01 drm/amd/display: reduce stack for
> dml32_CalculateSwathAndDETConfiguration
> 593eef8c1a5e drm/amd/display: reduce stack size in dcn32 dml (v2)
>
> https://gitlab.freedesktop.org/agd5f/linux/-/commits/amd-staging-drm-next/drivers/gpu/drm/amd/display/dc/dml/dcn32/display_mode_vba_32.c
Very good, thank you! I will test again on the next -next.
Thanx, Paul
> > Bisection located the commit shown below. Doing an allmodconfig build
> > on this commit reproduces the error, its parent builds fine.
> >
> > Thoughts?
> >
> > Thanx, Paul
> >
> >
> >
> > commit 3876a8b5e241081b2a519f848a65c00d8e6cd124
> > Author: Guenter Roeck
> > Date: Tue Jul 12 15:42:47 2022 -0700
> >
> > drm/amd/display: Enable building new display engine with KCOV enabled
> >
> > The new display engine uses floating point math, which is not supported
> > by KCOV. Commit 9d1d02ff3678 ("drm/amd/display: Don't build DCN1 when
> > kcov
> > is enabled") tried to work around the problem by disabling
> > CONFIG_DRM_AMD_DC_DCN if KCOV_INSTRUMENT_ALL and KCOV_ENABLE_COMPARISONS
> > are enabled. The result is that KCOV can not be enabled on systems which
> > require this display engine. A much simpler and less invasive solution
> > is
> > to disable KCOV selectively when compiling the display enagine while
> > keeping it enabled for the rest of the kernel.
> >
> > Fixes: 9d1d02ff3678 ("drm/amd/display: Don't build DCN1 when kcov is
> > enabled")
> > Cc: Arnd Bergmann
> > Cc: Leo Li
> > Reviewed-by: Harry Wentland
> > Signed-off-by: Guenter Roeck
> > Signed-off-by: Alex Deucher
> >
> > diff --git a/drivers/gpu/drm/amd/display/Kconfig
> > b/drivers/gpu/drm/amd/display/Kconfig
> > index b4029c0d5d8c5..96cbc87f7b6b8 100644
> > --- a/drivers/gpu/drm/amd/display/Kconfig
> > +++ b/drivers/gpu/drm/amd/display/Kconfig
> > @@ -6,7 +6,7 @@ config DRM_AMD_DC
> > bool "AMD DC - Enable new display engine"
> > default y
> > select SND_HDA_COMPONENT if SND_HDA_CORE
> > - select DRM_AMD_DC_DCN if (X86 || PPC64) && !(KCOV_INSTRUMENT_ALL &&
> > KCOV_ENABLE_COMPARISONS)
> > + select DRM_AMD_DC_DCN if (X86 || PPC64)
> > help
> > Choose this option if you want to use the new display engine
> > support for AMDGPU. This adds required support for Vega and
> > diff --git a/drivers/gpu/drm/amd/display/dc/Makefile
> > b/drivers/gpu/drm/amd/display/dc/Makefile
> > index 273f8f2c8e020..b9effadfc4bb7 100644
> > --- a/drivers/gpu/drm/amd/display/dc/Makefile
> > +++ b/drivers/gpu/drm/amd/display/dc/Makefile
> > @@ -25,6 +25,9 @@
> > DC_LIBS = basics bios dml clk_mgr dce gpio irq link virtual
> >
> > ifdef CONFIG_DRM_AMD_DC_DCN
> > +
> > +KCOV_INSTRUMENT := n
> > +
> > DC_LIBS += dcn20
> > DC_LIBS += dsc
> > DC_LIBS += dcn10
Stack-frame warnings in display_mode_vba_32.c
Hello!
I am seeing the following in allmodconfig builds of recent -next on x86:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c: In
function
‘DISPCLKDPPCLKDCFCLKDeepSleepPrefetchParametersWatermarksAndPerformanceCalculation’:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:1659:1:
error: the frame size of 2144 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]
1659 | }
| ^
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c: In
function ‘dml32_ModeSupportAndSystemConfigurationFull’:
drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn32/display_mode_vba_32.c:3799:1:
error: the frame size of 2480 bytes is larger than 2048 bytes
[-Werror=frame-larger-than=]
3799 | } // ModeSupportAndSystemConfigurationFull
| ^
Bisection located the commit shown below. Doing an allmodconfig build
on this commit reproduces the error, its parent builds fine.
Thoughts?
Thanx, Paul
commit 3876a8b5e241081b2a519f848a65c00d8e6cd124
Author: Guenter Roeck
Date: Tue Jul 12 15:42:47 2022 -0700
drm/amd/display: Enable building new display engine with KCOV enabled
The new display engine uses floating point math, which is not supported
by KCOV. Commit 9d1d02ff3678 ("drm/amd/display: Don't build DCN1 when kcov
is enabled") tried to work around the problem by disabling
CONFIG_DRM_AMD_DC_DCN if KCOV_INSTRUMENT_ALL and KCOV_ENABLE_COMPARISONS
are enabled. The result is that KCOV can not be enabled on systems which
require this display engine. A much simpler and less invasive solution is
to disable KCOV selectively when compiling the display enagine while
keeping it enabled for the rest of the kernel.
Fixes: 9d1d02ff3678 ("drm/amd/display: Don't build DCN1 when kcov is
enabled")
Cc: Arnd Bergmann
Cc: Leo Li
Reviewed-by: Harry Wentland
Signed-off-by: Guenter Roeck
Signed-off-by: Alex Deucher
diff --git a/drivers/gpu/drm/amd/display/Kconfig
b/drivers/gpu/drm/amd/display/Kconfig
index b4029c0d5d8c5..96cbc87f7b6b8 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -6,7 +6,7 @@ config DRM_AMD_DC
bool "AMD DC - Enable new display engine"
default y
select SND_HDA_COMPONENT if SND_HDA_CORE
- select DRM_AMD_DC_DCN if (X86 || PPC64) && !(KCOV_INSTRUMENT_ALL &&
KCOV_ENABLE_COMPARISONS)
+ select DRM_AMD_DC_DCN if (X86 || PPC64)
help
Choose this option if you want to use the new display engine
support for AMDGPU. This adds required support for Vega and
diff --git a/drivers/gpu/drm/amd/display/dc/Makefile
b/drivers/gpu/drm/amd/display/dc/Makefile
index 273f8f2c8e020..b9effadfc4bb7 100644
--- a/drivers/gpu/drm/amd/display/dc/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/Makefile
@@ -25,6 +25,9 @@
DC_LIBS = basics bios dml clk_mgr dce gpio irq link virtual
ifdef CONFIG_DRM_AMD_DC_DCN
+
+KCOV_INSTRUMENT := n
+
DC_LIBS += dcn20
DC_LIBS += dsc
DC_LIBS += dcn10
Re: CONFIG_ANDROID (was: rcu_sched detected expedited stalls in amdgpu after suspend)
On Thu, Jul 07, 2022 at 09:30:39AM +0200, Christian König wrote: > Am 06.07.22 um 22:42 schrieb Paul E. McKenney: > > On Wed, Jul 06, 2022 at 08:09:49PM +0200, Uladzislau Rezki wrote: > > > On Wed, Jul 06, 2022 at 10:58:36AM -0700, Paul E. McKenney wrote: > > > > On Wed, Jul 06, 2022 at 07:48:20PM +0200, Uladzislau Rezki wrote: > > > > > Hello. > > > > > > > > > > On Mon, Jul 04, 2022 at 01:30:50PM +0200, Christian König wrote: > > > > > > Hi guys, > > > > > > > > > > > > Am 28.06.22 um 22:11 schrieb Uladzislau Rezki: > > > > > > > > Excerpts from Paul E. McKenney's message of June 28, 2022 2:54 > > > > > > > > pm: > > > > > > > > > All you need to do to get the previous behavior is to add > > > > > > > > > something like > > > > > > > > > this to your defconfig file: > > > > > > > > > > > > > > > > > > CONFIG_RCU_EXP_CPU_STALL_TIMEOUT=21000 > > > > > > > > > > > > > > > > > > Any reason why this will not work for you? > > > > > > sorry for jumping in so later, I was on vacation for a week. > > > > > > > > > > > > Well when any RCU period is longer than 20ms and amdgpu in the > > > > > > backtrace my > > > > > > educated guess is that we messed up some timeout waiting for the hw. > > > > > > > > > > > > We usually do wait a few us, but it can be that somebody is waiting > > > > > > for ms > > > > > > instead. > > > > > > > > > > > > So there are some todos here as far as I can see and It would be > > > > > > helpful to > > > > > > get a cleaner backtrace if possible. > > > > > > > > > > > Actually CONFIG_ANDROID looks like is going to be removed, so the > > > > > CONFIG_RCU_EXP_CPU_STALL_TIMEOUT > > > > > will not have any dependencies on the CONFIG_ANDROID anymore: > > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2022%2F6%2F29%2F756&data=05%7C01%7Cchristian.koenig%40amd.com%7C8b36bcb4fe61475c0eb708da5f8ffce8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637927369274030797%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=eaK66spsbWVi2uRhcFK7eu4usgkHFZCSvErZxB%2F2npM%3D&reserved=0 > > > > But you can set the RCU_EXP_CPU_STALL_TIMEOUT Kconfig option, if you > > > > wish. Setting this option to 20 will get you the behavior previously > > > > obtained by setting the now-defunct ANDROID Kconfig option. > > > > > > > Right. Or over boot parameter. So for us it is not a big issue :) > > Specifically rcupdate.rcu_exp_cpu_stall_timeout, for those just now > > tuning in. ;-) > > I was just about to write a response asking for that :) > > Thanks, I will suggest to our QA to add this parameter while doing some > tests. Very good! Please let me know how it goes. Thanx, Paul
Re: CONFIG_ANDROID (was: rcu_sched detected expedited stalls in amdgpu after suspend)
On Wed, Jul 06, 2022 at 08:09:49PM +0200, Uladzislau Rezki wrote: > On Wed, Jul 06, 2022 at 10:58:36AM -0700, Paul E. McKenney wrote: > > On Wed, Jul 06, 2022 at 07:48:20PM +0200, Uladzislau Rezki wrote: > > > Hello. > > > > > > On Mon, Jul 04, 2022 at 01:30:50PM +0200, Christian König wrote: > > > > Hi guys, > > > > > > > > Am 28.06.22 um 22:11 schrieb Uladzislau Rezki: > > > > > > Excerpts from Paul E. McKenney's message of June 28, 2022 2:54 pm: > > > > > > > All you need to do to get the previous behavior is to add > > > > > > > something like > > > > > > > this to your defconfig file: > > > > > > > > > > > > > > CONFIG_RCU_EXP_CPU_STALL_TIMEOUT=21000 > > > > > > > > > > > > > > Any reason why this will not work for you? > > > > > > > > sorry for jumping in so later, I was on vacation for a week. > > > > > > > > Well when any RCU period is longer than 20ms and amdgpu in the > > > > backtrace my > > > > educated guess is that we messed up some timeout waiting for the hw. > > > > > > > > We usually do wait a few us, but it can be that somebody is waiting for > > > > ms > > > > instead. > > > > > > > > So there are some todos here as far as I can see and It would be > > > > helpful to > > > > get a cleaner backtrace if possible. > > > > > > > Actually CONFIG_ANDROID looks like is going to be removed, so the > > > CONFIG_RCU_EXP_CPU_STALL_TIMEOUT > > > will not have any dependencies on the CONFIG_ANDROID anymore: > > > > > > https://lkml.org/lkml/2022/6/29/756 > > > > But you can set the RCU_EXP_CPU_STALL_TIMEOUT Kconfig option, if you > > wish. Setting this option to 20 will get you the behavior previously > > obtained by setting the now-defunct ANDROID Kconfig option. > > > Right. Or over boot parameter. So for us it is not a big issue :) Specifically rcupdate.rcu_exp_cpu_stall_timeout, for those just now tuning in. ;-) Thanx, Paul
Re: CONFIG_ANDROID (was: rcu_sched detected expedited stalls in amdgpu after suspend)
On Wed, Jul 06, 2022 at 07:48:20PM +0200, Uladzislau Rezki wrote: > Hello. > > On Mon, Jul 04, 2022 at 01:30:50PM +0200, Christian König wrote: > > Hi guys, > > > > Am 28.06.22 um 22:11 schrieb Uladzislau Rezki: > > > > Excerpts from Paul E. McKenney's message of June 28, 2022 2:54 pm: > > > > > All you need to do to get the previous behavior is to add something > > > > > like > > > > > this to your defconfig file: > > > > > > > > > > CONFIG_RCU_EXP_CPU_STALL_TIMEOUT=21000 > > > > > > > > > > Any reason why this will not work for you? > > > > sorry for jumping in so later, I was on vacation for a week. > > > > Well when any RCU period is longer than 20ms and amdgpu in the backtrace my > > educated guess is that we messed up some timeout waiting for the hw. > > > > We usually do wait a few us, but it can be that somebody is waiting for ms > > instead. > > > > So there are some todos here as far as I can see and It would be helpful to > > get a cleaner backtrace if possible. > > > Actually CONFIG_ANDROID looks like is going to be removed, so the > CONFIG_RCU_EXP_CPU_STALL_TIMEOUT > will not have any dependencies on the CONFIG_ANDROID anymore: > > https://lkml.org/lkml/2022/6/29/756 But you can set the RCU_EXP_CPU_STALL_TIMEOUT Kconfig option, if you wish. Setting this option to 20 will get you the behavior previously obtained by setting the now-defunct ANDROID Kconfig option. Thanx, Paul
Re: CONFIG_ANDROID (was: rcu_sched detected expedited stalls in amdgpu after suspend)
On Tue, Jun 28, 2022 at 11:02:40AM -0400, Alex Xu (Hello71) wrote: > Excerpts from Paul E. McKenney's message of June 28, 2022 12:12 am: > > On Mon, Jun 27, 2022 at 09:50:53PM -0400, Alex Xu (Hello71) wrote: > >> Ah, I see. I have selected the default value for > >> CONFIG_RCU_EXP_CPU_STALL_TIMEOUT, but that is 20 if ANDROID. I am not > >> using Android; I'm not sure there exist Android devices with AMD GPUs. > >> However, I have set CONFIG_ANDROID=y in order to use > >> ANDROID_BINDER_IPC=m for emulation. > >> > >> In general, I think CONFIG_ANDROID is not a reliable method for > >> detecting if the kernel is for an Android device; for example, Fedora > >> sets CONFIG_ANDROID, but (AFAIK) its kernel is not intended for use with > >> Android userspace. > >> > >> On the other hand, it's not clear to me why the value 20 should be for > >> Android only anyways. If, as you say in > >> https://lore.kernel.org/lkml/20220216195508.GM4285@paulmck-ThinkPad-P17-Gen-1/, > >> it is related to the size of the system, perhaps some other heuristic > >> would be more appropriate. > > > > It is related to the fact that quite a few Android guys want these > > 20-millisecond short-timeout expedited RCU CPU stall warnings, but no one > > else does. Not yet anyway. > > > > And let's face it, the intent and purpose of CONFIG_ANDROID=y is extremely > > straightforward and unmistakeable. So perhaps people not running Android > > devices but wanting a little bit of the Android functionality should do > > something other than setting CONFIG_ANDROID=y in their .config files. Me, > > I am surprised that it took this long for something like this to bite you. > > > > But just out of curiosity, what would you suggest instead? > > Both Debian and Fedora set CONFIG_ANDROID, specifically for binder. If > major distro vendors are consistently making this "mistake", then > perhaps the problem is elsewhere. > > In my own opinion, assuming that binderfs means Android vendor is not a > good assumption. The ANDROID help says: > > > Enable support for various drivers needed on the Android platform > > It doesn't say "Enable only if building an Android device", or "Enable > only if you are Google". Isn't the traditional Linux philosophy a > collection of pieces to be assembled, without gratuitous hidden > dependencies? For example, [0] removes the unnecessary Android > dependency, it doesn't block the whole thing with "depends on ANDROID". > > It seems to me that the proper way to set some configuration for Android > kernels is or should be to ask the Android kernel config maintainers, > not to set it based on an upstream kernel option. There is, after all, > no CONFIG_FEDORA or CONFIG_UBUNTU or CONFIG_HANNAH_MONTANA. > > WireGuard and random also use CONFIG_ANDROID in a similar "proxy" way as > rcu, there to see if suspends are "frequent". This seems dubious for the > same reasons. > > I wonder if it might be time to retire CONFIG_ANDROID: the only > remaining driver covered is binder, which originates from Android but > is no longer used exclusively on Android systems. Like ufs-qcom, binder > is no longer used exclusively on Android devices; it is also used for > Android device emulators, which might be used on Android-like mobile > devices, or might not. > > My understanding is that both Android and upstream kernel developers > intend to add no more Android-specific drivers, so binder should be the > only one covered for the foreseeable future. Thank you for the perspective, but you never did suggest an alternative. So here is is what I suggest given the current setup: config RCU_EXP_CPU_STALL_TIMEOUT int "Expedited RCU CPU stall timeout in milliseconds" depends on RCU_STALL_COMMON range 0 21000 default 20 if ANDROID default 0 if !ANDROID help If a given expedited RCU grace period extends more than the specified number of milliseconds, a CPU stall warning is printed. If the RCU grace period persists, additional CPU stall warnings are printed at more widely spaced intervals. A value of zero says to use the RCU_CPU_STALL_TIMEOUT value converted from seconds to milliseconds. The default, and only the default, is controlled by ANDROID. All you need to do to get the previous behavior is to add something like this to your defconfig file: CONFIG_RCU_EXP_CPU_STALL_TIMEOUT=21000 Any reason why this will not work for you? > > For that matter, why the private reply? > > Mail client issues, not intentional. Lists re-added, plus Android, > WireGuard, and random. Thank you! Thanx, Paul > Thanks, > Alex. > > [0] https://lore.kernel.org/all/20220321151853.24138-1-k...@kernel.org/
Re: rcu_sched detected expedited stalls in amdgpu after suspend
On Mon, Jun 27, 2022 at 03:22:24PM -0400, Alex Xu (Hello71) wrote:
> Hi,
>
> Since Linux 5.19-ish, I consistently get these types of errors when
> resuming from S3:
>
> [15652.909157] rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks:
> { 11-... } 7 jiffies s: 9981 root: 0x800/.
> [15652.909162] rcu: blocking rcu_node structures (internal RCU debug):
> [15652.909163] Task dump for CPU 11:
> [15652.909164] task:kworker/u24:65 state:R running task stack:0
> pid:210218 ppid: 2 flags:0x4008
> [15652.909167] Workqueue: events_unbound async_run_entry_fn
> [15652.909172] Call Trace:
> [15652.909173]
> [15652.909174] ? atom_get_src_int+0x38e/0x680
> [15652.909179] ? atom_op_test+0x67/0x190
> [15652.909181] ? amdgpu_atom_execute_table_locked+0x19a/0x300
> [15652.909184] ? atom_op_calltable+0xb1/0x110
> [15652.909186] ? amdgpu_atom_execute_table_locked+0x19a/0x300
> [15652.909189] ? atom_op_calltable+0xb1/0x110
> [15652.909191] ? amdgpu_atom_execute_table_locked+0x19a/0x300
> [15652.909193] ? __switch_to+0x137/0x440
> [15652.909195] ? amdgpu_atom_asic_init+0xe0/0x100
> [15652.909198] ? pci_bus_read_config_dword+0x36/0x50
> [15652.909201] ? amdgpu_device_resume+0x10b/0x3e0
> [15652.909203] ? amdgpu_pmops_resume+0x32/0x60
> [15652.909204] ? pci_pm_suspend+0x2b0/0x2b0
> [15652.909206] ? dpm_run_callback+0x35/0x1f0
> [15652.909209] ? device_resume+0x1ca/0x220
> [15652.909211] ? async_resume+0x19/0xe0
> [15652.909213] ? async_run_entry_fn+0x33/0x120
> [15652.909215] ? process_one_work+0x1d6/0x350
> [15652.909218] ? worker_thread+0x24d/0x480
> [15652.909220] ? kthread+0x137/0x150
> [15652.909221] ? worker_clr_flags+0x40/0x40
> [15652.909224] ? kthread_blkcg+0x30/0x30
> [15652.909226] ? ret_from_fork+0x22/0x30
> [15652.909227]
> [15653.015808] rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks:
> { 11-... } 7 jiffies s: 9985 root: 0x800/.
> [15653.015812] rcu: blocking rcu_node structures (internal RCU debug):
> [15653.015813] Task dump for CPU 11:
> [15653.015813] task:kworker/u24:65 state:R running task stack:0
> pid:210218 ppid: 2 flags:0x4008
> [15653.015816] Workqueue: events_unbound async_run_entry_fn
> [15653.015820] Call Trace:
> [15653.015820]
> [15653.015821] ? amdgpu_cgs_read_register+0x10/0x10
> [15653.015825] ? smu7_copy_bytes_to_smc+0xd4/0x200
> [15653.015828] ? polaris10_program_memory_timing_parameters+0x195/0x1b0
> [15653.015831] ? sysvec_apic_timer_interrupt+0xa/0x80
> [15653.015834] ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
> [15653.015836] ? amdgpu_cgs_destroy_device+0x10/0x10
> [15653.015839] ? sysvec_apic_timer_interrupt+0xa/0x80
> [15653.015841] ? asm_sysvec_apic_timer_interrupt+0x1b/0x20
> [15653.015843] ? amdgpu_cgs_destroy_device+0x10/0x10
> [15653.015846] ? amdgpu_device_rreg+0x8f/0xd0
> [15653.015847] ? phm_wait_for_register_unequal+0x99/0xd0
> [15653.015850] ? smu7_send_msg_to_smc+0x95/0x130
> [15653.015853] ? smum_send_msg_to_smc+0x5d/0xa0
> [15653.015854] ? amdgpu_cgs_read_ind_register+0xa0/0xa0
> [15653.015857] ? smu7_enable_dpm_tasks+0x241f/0x28c0
> [15653.015859] ? hwmgr_resume+0x31/0x70
> [15653.015861] ? amdgpu_device_resume+0x1fa/0x3e0
> [15653.015863] ? amdgpu_pmops_resume+0x32/0x60
> [15653.015864] ? pci_pm_suspend+0x2b0/0x2b0
> [15653.015866] ? dpm_run_callback+0x35/0x1f0
> [15653.015868] ? device_resume+0x1ca/0x220
> [15653.015870] ? async_resume+0x19/0xe0
> [15653.015872] ? async_run_entry_fn+0x33/0x120
> [15653.015874] ? process_one_work+0x1d6/0x350
> [15653.015877] ? worker_thread+0x24d/0x480
> [15653.015878] ? kthread+0x137/0x150
> [15653.015880] ? worker_clr_flags+0x40/0x40
> [15653.015882] ? kthread_blkcg+0x30/0x30
> [15653.015884] ? ret_from_fork+0x22/0x30
> [15653.015886]
>
> I have not noticed any resulting problems. I am reporting this in the
> hope that it is easy to fix the issue and remove the error messages
> which may obscure some future problem.
The usual way this happens is for a task to be spinning. In this case,
that might be due to excessive lock contention on async_lock within the
async_run_entry_fn() function. Or perhaps the problem is in one of the
functions preceded by "?" above.
One way to debug this is to place trace_printk()s or similar in the
functions called out above to track it down.
I bet that the reason this showed up in v5.19 is this guy:
28b3ae426598 ("rcu: Introduce CONFIG_RCU_EXP_CPU_STALL_TIMEOUT")
So do you have CONFIG_RCU_EXP_CPU_STALL_TIMEOUT set to some small
number of milliseconds? If so, you can override this by adjusting the
CONFIG_RCU_EXP_CPU_STALL_TIMEOUT Kconfig option or by booting with a
longer timeout via the rcupdate.rcu_exp_cpu_stall_timeout= kernel boot
parameter.
But if you are running on an Android platform, Uladzislau will be
interested in addressing the underlying issue, so I have added him on CC.
Thanx, Paul
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 09, 2019 at 02:04:11PM -0400, Mathieu Desnoyers wrote: > - On Apr 9, 2019, at 1:55 PM, paulmck paul...@linux.ibm.com wrote: > [...] > > The current state is not horrible, so my thought would be to give it > > some time to see if better thoughts arise. > > > > Either way, cleanup_srcu_struct() keeps its current checks for callbacks > > still being in flight, which is why I believe that the current state is > > not horrible. ;-) > > In that case, I think the comment above cleanup_srcu_struct_quiesced() in > include/linux/srcu.h needs to be updated to cover situations where API > users statically define a SRCU domain in a module and intend to unload > that module. > > Given that we end up doing the allocation/cleanup under the hood, the > API users don't interact with init_srcu_struct() nor cleanup_srcu_struct(), > so it's not obvious that this comment also applies to them. Actually, it turned out that cleanup_srcu_struct_quiesced() is extremely hard to use correctly, and maybe even impossible to use correctly. So cleanup_srcu_struct_quiesced has been eliminated in current -rcu. Thanx, Paul > Thanks, > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 09, 2019 at 12:45:25PM -0400, Mathieu Desnoyers wrote: > - On Apr 9, 2019, at 12:40 PM, paulmck paul...@linux.ibm.com wrote: > > > 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 +, 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) > >> >> >> >> >> > >> >> > &
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 +, 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 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 > >> >> > >> >> > 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? > >> >> > >> > >
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
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 > >> > >> >> > 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 sta
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Sun, Apr 07, 2019 at 08:36:46PM -0400, Joel Fernandes wrote: > On Sun, Apr 07, 2019 at 10:05:14AM -0700, Paul E. McKenney wrote: > > On Sun, Apr 07, 2019 at 03:46:13PM +, 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
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
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?
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 as suggested by Joel Fernandes. ]
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 = .;\
} \
\
.rodata1 : AT(ADDR(.rodata1) - LOAD_OFFSET) { \
diff --git a/include/linux/module.h b/include/linux/module.h
index 5bf5dcd91009..921443a026dd 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -21,6 +21,7 @
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Sun, Apr 07, 2019 at 03:46:13PM +, 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. > 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! > > And may I have your Tested-by? > > Absolutely, please do and thanks! Done, and thank you for giving it a go! Thanx, Paul > - 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. Howeve
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
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Sat, Apr 06, 2019 at 01:33:27PM +, Joel Fernandes wrote:
> 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.
> > > >
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Sat, Apr 06, 2019 at 07:06:13PM -0400, Joel Fernandes wrote:
> 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.
> > > >
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
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 > > >> >> > 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. If there are call_srcu() callbacks outstanding, the module writer still needs the srcu_barrier() because otherwise callbacks arrive after the module text has gone, which will be disappoint the CPU when it tries fetching instructions that are no longer mapped. If there are no call_srcu() callbacks from that module, then there is no need for srcu_barrier() either way. So if an srcu_barrier() is needed, the module developer needs to supply it. Thanx, Paul ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC tip/core/rcu 3/4] drivers/gpu/drm/amd: Dynamically allocate kfd_processes_srcu
On Tue, Apr 02, 2019 at 05:40:54PM +, Kuehling, Felix wrote:
> On 2019-04-02 10:29 a.m., Paul E. McKenney wrote:
> > Having DEFINE_SRCU() or DEFINE_STATIC_SRCU() in a loadable module
> > requires that the size of the reserved region be increased, which is
> > not something we really want to be doing. This commit therefore removes
> > the DEFINE_STATIC_SRCU() from drivers/gpu/drm/amd/amdkfd/kfd_process.c in
> > favor of defining kfd_processes_srcu as a simple srcu_struct, initializing
> > it in amdgpu_amdkfd_init(), and cleaning it up in amdgpu_amdkfd_fini().
> >
> > Reported-by: kbuild test robot
> > Signed-off-by: Paul E. McKenney
> > Tested-by: kbuild test robot
> > Cc: Oded Gabbay
> > Cc: Alex Deucher
> > Cc: "Christian König" > Cc: "David (ChunMing) Zhou"
> > Cc: David Airlie
> > Cc: Daniel Vetter
> > Cc: Tejun Heo
> > Cc:
> > Cc:
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 +
> > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +-
> > 2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > index fe1d7368c1e6..eadb20dee867 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
> > @@ -28,6 +28,8 @@
> > #include
> > #include
> >
> > +extern struct srcu_struct kfd_processes_srcu;
> > +
> > static const unsigned int compute_vmid_bitmap = 0xFF00;
> >
> > /* Total memory size in system memory and all GPU VRAM. Used to
> > @@ -40,6 +42,8 @@ int amdgpu_amdkfd_init(void)
> > struct sysinfo si;
> > int ret;
> >
> > + ret = init_srcu_struct(&kfd_processes_srcu);
> > + WARN_ON(ret);
>
> kfd_processes_srcu only exists if kfd_process.c is compiled in. That
> depends on CONFIG_HSA_AMD. So this should at least move into #ifdef a
> few lines below.
>
> However, it would be cleaner to move this initialization into kfd_init
> in kfd_module.c, or better yet, into kfd_process_create_wq in
> kfd_process.c. Then kfd_process_create_wq should be renamed to something
> more generic, such as kfd_process_init.
>
>
> > si_meminfo(&si);
> > amdgpu_amdkfd_total_mem_size = si.totalram - si.totalhigh;
> > amdgpu_amdkfd_total_mem_size *= si.mem_unit;
> > @@ -57,6 +61,7 @@ int amdgpu_amdkfd_init(void)
> > void amdgpu_amdkfd_fini(void)
> > {
> > kgd2kfd_exit();
> > + cleanup_srcu_struct(&kfd_processes_srcu);
>
> Similarly, this would be cleaner in kfd_exit in kfd_module.c or
> kfd_process_destroy_wq in kfd_process.c, with that function similarly
> renamed to kfd_process_fini.
>
> I'm attaching a revised patch. It's only compile tested.
Thank you, Felix! I have reverted my original and applied your revised
version.
Thanx, Paul
> Regards,
> Felix
>
>
> > }
> >
> > void amdgpu_amdkfd_device_probe(struct amdgpu_device *adev)
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > index 4bdae78bab8e..98b694068b8a 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> > @@ -47,7 +47,7 @@ struct mm_struct;
> > DEFINE_HASHTABLE(kfd_processes_table, KFD_PROCESS_TABLE_SIZE);
> > static DEFINE_MUTEX(kfd_processes_mutex);
> >
> > -DEFINE_SRCU(kfd_processes_srcu);
> > +struct srcu_struct kfd_processes_srcu;
> >
> > /* For process termination handling */
> > static struct workqueue_struct *kfd_process_wq;
> From 5857a9aa63957a5755ff81ae5c46533bca408c12 Mon Sep 17 00:00:00 2001
> From: "Paul E. McKenney"
> Date: Tue, 2 Apr 2019 07:29:32 -0700
> Subject: [PATCH 1/1] drivers/gpu/drm/amd: Dynamically allocate
> kfd_processes_srcu v2
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Having DEFINE_SRCU() or DEFINE_STATIC_SRCU() in a loadable module
> requires that the size of the reserved region be increased, which is
> not something we really want to be doing. This commit therefore removes
> the DEFINE_STATIC_SRCU() from drivers/gpu/drm/amd/amdkfd/kfd_process.c in
> favor of defining kfd_processes_srcu as a simple srcu_struct, initializing
> it in kfd_process_init(), and cleaning it up in kfd_process_fini().
>
> v2 (Felix Kuehling): Move srcu init and cleanup into kf
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
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("__tracepoints_ptrs"))) = \
> &__tracepoint_##name
> #endif
>
> [...]
>
> #define DEFINE_TRACE_FN(name, reg, unreg)\
> static const char __tpstrtab_##name[]\
> __attribute__((section("__tracepoints_s
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
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 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.
Thanx, Paul
> Thanks,
>
> Mathieu
>
>
> >
> > Thanx, Paul
> >
> >> Thanks,
> >>
> >> Mathieu
> >>
> >>
> >> >
> >> > This series consist of the following:
> >> >
> >> > 1. Dynamically allocate dax_srcu.
> >> >
> >> > 2. Dynamically allocate drm_unplug_srcu.
> >> >
> >> > 3. Dynamically allocate kfd_processes_srcu.
> >> >
> >> > These build and have been subjected to 0day testing, but might also need
> >> > testing by someone having the requisite hardware.
> >> >
> >> > Thanx, Paul
> >> >
> >> >
> >> >
> >> > drivers/dax/super.c| 10 +-
> >> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |5 +++
> >> > drivers/gpu/drm/amd/amdkfd/kfd_process.c |2 -
> >> > drivers/gpu/drm/drm_drv.c |8
> >> > include/linux/srcutree.h | 19 +--
> >> > kernel/rcu/rcuperf.c | 40
> >> > +++-
> >> > kernel/rcu/rcutorture.c| 48
> >> > +
> >> > 7 files changed, 105 insertions(+), 27 deletions(-)
> >>
> >> --
> >> Mathieu Desnoyers
> >> EfficiOS Inc.
> >> http://www.efficios.com
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
On Tue, Apr 02, 2019 at 02:40:54PM -0400, Joel Fernandes wrote: > 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? The percpu/module reservation discussed in this thread: https://lore.kernel.org/lkml/c72402f2-967e-cd56-99d8-9139c9e7f...@google.com/T/#mbcacf60ddc0b3fd6e831a3ea71efc90da359a3bf For non-modules, global per-CPU variables are statically allocated. For modules, they must be dynamically allocated at modprobe time, and their size is set by PERCPU_MODULE_RESERVE. Thanx, Paul ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
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? Thanx, Paul > Thanks, > > Mathieu > > > > > > This series consist of the following: > > > > 1. Dynamically allocate dax_srcu. > > > > 2. Dynamically allocate drm_unplug_srcu. > > > > 3. Dynamically allocate kfd_processes_srcu. > > > > These build and have been subjected to 0day testing, but might also need > > testing by someone having the requisite hardware. > > > > Thanx, Paul > > > > > > > > drivers/dax/super.c| 10 +- > > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |5 +++ > > drivers/gpu/drm/amd/amdkfd/kfd_process.c |2 - > > drivers/gpu/drm/drm_drv.c |8 > > include/linux/srcutree.h | 19 +-- > > kernel/rcu/rcuperf.c | 40 +++- > > kernel/rcu/rcutorture.c| 48 > > + > > 7 files changed, 105 insertions(+), 27 deletions(-) > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com > ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH RFC tip/core/rcu 3/4] drivers/gpu/drm/amd: Dynamically allocate kfd_processes_srcu
Having DEFINE_SRCU() or DEFINE_STATIC_SRCU() in a loadable module
requires that the size of the reserved region be increased, which is
not something we really want to be doing. This commit therefore removes
the DEFINE_STATIC_SRCU() from drivers/gpu/drm/amd/amdkfd/kfd_process.c in
favor of defining kfd_processes_srcu as a simple srcu_struct, initializing
it in amdgpu_amdkfd_init(), and cleaning it up in amdgpu_amdkfd_fini().
Reported-by: kbuild test robot
Signed-off-by: Paul E. McKenney
Tested-by: kbuild test robot
Cc: Oded Gabbay
Cc: Alex Deucher
Cc: "Christian König"
Cc: David Airlie
Cc: Daniel Vetter
Cc: Tejun Heo
Cc:
Cc:
---
drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 5 +
drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
index fe1d7368c1e6..eadb20dee867 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c
@@ -28,6 +28,8 @@
#include
#include
+extern struct srcu_struct kfd_processes_srcu;
+
static const unsigned int compute_vmid_bitmap = 0xFF00;
/* Total memory size in system memory and all GPU VRAM. Used to
@@ -40,6 +42,8 @@ int amdgpu_amdkfd_init(void)
struct sysinfo si;
int ret;
+ ret = init_srcu_struct(&kfd_processes_srcu);
+ WARN_ON(ret);
si_meminfo(&si);
amdgpu_amdkfd_total_mem_size = si.totalram - si.totalhigh;
amdgpu_amdkfd_total_mem_size *= si.mem_unit;
@@ -57,6 +61,7 @@ int amdgpu_amdkfd_init(void)
void amdgpu_amdkfd_fini(void)
{
kgd2kfd_exit();
+ cleanup_srcu_struct(&kfd_processes_srcu);
}
void amdgpu_amdkfd_device_probe(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 4bdae78bab8e..98b694068b8a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -47,7 +47,7 @@ struct mm_struct;
DEFINE_HASHTABLE(kfd_processes_table, KFD_PROCESS_TABLE_SIZE);
static DEFINE_MUTEX(kfd_processes_mutex);
-DEFINE_SRCU(kfd_processes_srcu);
+struct srcu_struct kfd_processes_srcu;
/* For process termination handling */
static struct workqueue_struct *kfd_process_wq;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH RFC tip/core/rcu 0/4] Forbid static SRCU use in modules
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 series consist of the following: 1. Dynamically allocate dax_srcu. 2. Dynamically allocate drm_unplug_srcu. 3. Dynamically allocate kfd_processes_srcu. These build and have been subjected to 0day testing, but might also need testing by someone having the requisite hardware. Thanx, Paul drivers/dax/super.c| 10 +- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c |5 +++ drivers/gpu/drm/amd/amdkfd/kfd_process.c |2 - drivers/gpu/drm/drm_drv.c |8 include/linux/srcutree.h | 19 +-- kernel/rcu/rcuperf.c | 40 +++- kernel/rcu/rcutorture.c| 48 + 7 files changed, 105 insertions(+), 27 deletions(-) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
