* v5.7: new core kernel option missing help text
@ 2020-06-03 17:31 Russell King - ARM Linux admin
2020-06-03 18:00 ` Valentin Schneider
0 siblings, 1 reply; 17+ messages in thread
From: Russell King - ARM Linux admin @ 2020-06-03 17:31 UTC (permalink / raw)
To: Thara Gopinath; +Cc: Vincent Guittot, linux-kernel
Hi,
A new kernel configuration option ("SCHED_THERMAL_PRESSURE") was
recently added, but has no help text. This is most unhelpful when
trying to configure the kernel, since one does not know what the
effect of answering yes or no to this option would be.
Please supply a proper help text when adding core kernel options
so that people can make an informed decision when answering the
prompt, rather than just guessing.
One suggestion on IRC has been that this option has something to
do with climate change.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: v5.7: new core kernel option missing help text 2020-06-03 17:31 v5.7: new core kernel option missing help text Russell King - ARM Linux admin @ 2020-06-03 18:00 ` Valentin Schneider 2020-06-03 18:45 ` Russell King - ARM Linux admin 0 siblings, 1 reply; 17+ messages in thread From: Valentin Schneider @ 2020-06-03 18:00 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Thara Gopinath, Vincent Guittot, linux-kernel On 03/06/20 18:31, Russell King - ARM Linux admin wrote: > Hi, > > A new kernel configuration option ("SCHED_THERMAL_PRESSURE") was > recently added, but has no help text. This is most unhelpful when > trying to configure the kernel, since one does not know what the > effect of answering yes or no to this option would be. > > Please supply a proper help text when adding core kernel options > so that people can make an informed decision when answering the > prompt, rather than just guessing. > Right; does the below look good enough? --- diff --git a/init/Kconfig b/init/Kconfig index 74a5ac65644f..f40cf852d00a 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -441,6 +441,10 @@ config HAVE_SCHED_AVG_IRQ config SCHED_THERMAL_PRESSURE bool "Enable periodic averaging of thermal pressure" depends on SMP + help + This option allows the scheduler to be aware of CPU thermal throttling + (i.e. thermal pressure), providing arch_scale_thermal_pressure() is + implemented. config BSD_PROCESS_ACCT bool "BSD Process Accounting" --- ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-03 18:00 ` Valentin Schneider @ 2020-06-03 18:45 ` Russell King - ARM Linux admin 2020-06-03 19:24 ` Vincent Guittot 0 siblings, 1 reply; 17+ messages in thread From: Russell King - ARM Linux admin @ 2020-06-03 18:45 UTC (permalink / raw) To: Valentin Schneider; +Cc: Thara Gopinath, Vincent Guittot, linux-kernel On Wed, Jun 03, 2020 at 07:00:26PM +0100, Valentin Schneider wrote: > > On 03/06/20 18:31, Russell King - ARM Linux admin wrote: > > Hi, > > > > A new kernel configuration option ("SCHED_THERMAL_PRESSURE") was > > recently added, but has no help text. This is most unhelpful when > > trying to configure the kernel, since one does not know what the > > effect of answering yes or no to this option would be. > > > > Please supply a proper help text when adding core kernel options > > so that people can make an informed decision when answering the > > prompt, rather than just guessing. > > > > Right; does the below look good enough? It's a start. I'm still wondering whether I should answer yes or no for the platforms I'm building for. So far, all I've found is: arch/arm/include/asm/topology.h:#define arch_scale_thermal_pressure topology_get_thermal_pressure which really doesn't tell me anything about this. So I'm still in the dark. I guess topology_get_thermal_pressure is provided by something in drivers/ which will be conditional on some driver or something. > > --- > diff --git a/init/Kconfig b/init/Kconfig > index 74a5ac65644f..f40cf852d00a 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -441,6 +441,10 @@ config HAVE_SCHED_AVG_IRQ > config SCHED_THERMAL_PRESSURE > bool "Enable periodic averaging of thermal pressure" > depends on SMP > + help > + This option allows the scheduler to be aware of CPU thermal throttling > + (i.e. thermal pressure), providing arch_scale_thermal_pressure() is > + implemented. > > config BSD_PROCESS_ACCT > bool "BSD Process Accounting" > --- > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-03 18:45 ` Russell King - ARM Linux admin @ 2020-06-03 19:24 ` Vincent Guittot 2020-06-03 19:58 ` Russell King - ARM Linux admin 0 siblings, 1 reply; 17+ messages in thread From: Vincent Guittot @ 2020-06-03 19:24 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Valentin Schneider, Thara Gopinath, linux-kernel On Wed, 3 Jun 2020 at 20:45, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > > On Wed, Jun 03, 2020 at 07:00:26PM +0100, Valentin Schneider wrote: > > > > On 03/06/20 18:31, Russell King - ARM Linux admin wrote: > > > Hi, > > > > > > A new kernel configuration option ("SCHED_THERMAL_PRESSURE") was > > > recently added, but has no help text. This is most unhelpful when > > > trying to configure the kernel, since one does not know what the > > > effect of answering yes or no to this option would be. > > > > > > Please supply a proper help text when adding core kernel options > > > so that people can make an informed decision when answering the > > > prompt, rather than just guessing. > > > > > > > Right; does the below look good enough? > > It's a start. I'm still wondering whether I should answer yes or no > for the platforms I'm building for. > > So far, all I've found is: > > arch/arm/include/asm/topology.h:#define arch_scale_thermal_pressure topology_get_thermal_pressure > > which really doesn't tell me anything about this. So I'm still in > the dark. > > I guess topology_get_thermal_pressure is provided by something in > drivers/ which will be conditional on some driver or something. You need cpufreq_cooling device to make it useful and only for SMP I don't think that this should not be user configurable because even with the description above, it is not easy to choose. This should be set by the driver that implement the feature which is only cpufreq cooling device for now it > > > > > --- > > diff --git a/init/Kconfig b/init/Kconfig > > index 74a5ac65644f..f40cf852d00a 100644 > > --- a/init/Kconfig > > +++ b/init/Kconfig > > @@ -441,6 +441,10 @@ config HAVE_SCHED_AVG_IRQ > > config SCHED_THERMAL_PRESSURE > > bool "Enable periodic averaging of thermal pressure" > > depends on SMP > > + help > > + This option allows the scheduler to be aware of CPU thermal throttling > > + (i.e. thermal pressure), providing arch_scale_thermal_pressure() is > > + implemented. > > > > config BSD_PROCESS_ACCT > > bool "BSD Process Accounting" > > --- > > > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-03 19:24 ` Vincent Guittot @ 2020-06-03 19:58 ` Russell King - ARM Linux admin 2020-06-03 20:22 ` Vincent Guittot 2020-06-03 20:25 ` Valentin Schneider 0 siblings, 2 replies; 17+ messages in thread From: Russell King - ARM Linux admin @ 2020-06-03 19:58 UTC (permalink / raw) To: Vincent Guittot; +Cc: Valentin Schneider, Thara Gopinath, linux-kernel On Wed, Jun 03, 2020 at 09:24:56PM +0200, Vincent Guittot wrote: > On Wed, 3 Jun 2020 at 20:45, Russell King - ARM Linux admin > <linux@armlinux.org.uk> wrote: > > It's a start. I'm still wondering whether I should answer yes or no > > for the platforms I'm building for. > > > > So far, all I've found is: > > > > arch/arm/include/asm/topology.h:#define arch_scale_thermal_pressure topology_get_thermal_pressure > > > > which really doesn't tell me anything about this. So I'm still in > > the dark. > > > > I guess topology_get_thermal_pressure is provided by something in > > drivers/ which will be conditional on some driver or something. > > You need cpufreq_cooling device to make it useful and only for SMP > I don't think that this should not be user configurable because even > with the description above, it is not easy to choose. > This should be set by the driver that implement the feature which is > only cpufreq cooling device for now it As I have CONFIG_CPU_FREQ_THERMAL=y in my config, I'm guessing (and it's only a guess) that I should say y to SCHED_THERMAL_PRESSURE ? > > > + help > > > + This option allows the scheduler to be aware of CPU thermal throttling > > > + (i.e. thermal pressure), providing arch_scale_thermal_pressure() is > > > + implemented. Is this feature documented in terms of what it does? Do I assume that as the thermal trip points start tripping, that has an influence on the scheduler? Or is it the case that the scheduler is wanting to know when the cpu frequency changes? Grepping for "thermal" in Documentation/scheduler brings up nothing. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-03 19:58 ` Russell King - ARM Linux admin @ 2020-06-03 20:22 ` Vincent Guittot 2020-06-03 20:25 ` Valentin Schneider 1 sibling, 0 replies; 17+ messages in thread From: Vincent Guittot @ 2020-06-03 20:22 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Valentin Schneider, Thara Gopinath, linux-kernel On Wed, 3 Jun 2020 at 21:59, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote: > > On Wed, Jun 03, 2020 at 09:24:56PM +0200, Vincent Guittot wrote: > > On Wed, 3 Jun 2020 at 20:45, Russell King - ARM Linux admin > > <linux@armlinux.org.uk> wrote: > > > It's a start. I'm still wondering whether I should answer yes or no > > > for the platforms I'm building for. > > > > > > So far, all I've found is: > > > > > > arch/arm/include/asm/topology.h:#define arch_scale_thermal_pressure topology_get_thermal_pressure > > > > > > which really doesn't tell me anything about this. So I'm still in > > > the dark. > > > > > > I guess topology_get_thermal_pressure is provided by something in > > > drivers/ which will be conditional on some driver or something. > > > > You need cpufreq_cooling device to make it useful and only for SMP > > I don't think that this should not be user configurable because even > > with the description above, it is not easy to choose. > > This should be set by the driver that implement the feature which is > > only cpufreq cooling device for now it > > As I have CONFIG_CPU_FREQ_THERMAL=y in my config, I'm guessing (and it's > only a guess) that I should say y to SCHED_THERMAL_PRESSURE ? yes, you're right > > > > > + help > > > > + This option allows the scheduler to be aware of CPU thermal throttling > > > > + (i.e. thermal pressure), providing arch_scale_thermal_pressure() is > > > > + implemented. > > Is this feature documented in terms of what it does? Do I assume that > as the thermal trip points start tripping, that has an influence on > the scheduler? Or is it the case that the scheduler is wanting to > know when the cpu frequency changes? When the thermal trip points start tripping, we take into account the decrease of the compute capacity of the CU. This reduced capacity is used instead of max capacity when we balance the tasks between CPUs. A similar mechanism is used to account the time "stolen" by IRQ or RT/DL tasks to CFS tasks > > Grepping for "thermal" in Documentation/scheduler brings up nothing. John Mathew sent a patch to add documentation: https://lkml.org/lkml/2020/5/14/290 > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-03 19:58 ` Russell King - ARM Linux admin 2020-06-03 20:22 ` Vincent Guittot @ 2020-06-03 20:25 ` Valentin Schneider 2020-06-04 0:48 ` Thara Gopinath 1 sibling, 1 reply; 17+ messages in thread From: Valentin Schneider @ 2020-06-03 20:25 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Vincent Guittot, Thara Gopinath, linux-kernel On 03/06/20 20:58, Russell King - ARM Linux admin wrote: > On Wed, Jun 03, 2020 at 09:24:56PM +0200, Vincent Guittot wrote: >> On Wed, 3 Jun 2020 at 20:45, Russell King - ARM Linux admin >> <linux@armlinux.org.uk> wrote: >> > It's a start. I'm still wondering whether I should answer yes or no >> > for the platforms I'm building for. >> > >> > So far, all I've found is: >> > >> > arch/arm/include/asm/topology.h:#define arch_scale_thermal_pressure topology_get_thermal_pressure >> > >> > which really doesn't tell me anything about this. So I'm still in >> > the dark. >> > >> > I guess topology_get_thermal_pressure is provided by something in >> > drivers/ which will be conditional on some driver or something. >> >> You need cpufreq_cooling device to make it useful and only for SMP >> I don't think that this should not be user configurable because even >> with the description above, it is not easy to choose. >> This should be set by the driver that implement the feature which is >> only cpufreq cooling device for now it > > As I have CONFIG_CPU_FREQ_THERMAL=y in my config, I'm guessing (and it's > only a guess) that I should say y to SCHED_THERMAL_PRESSURE ? > arm and arm64 implement arch_scale_thermal_pressure(); the actual implementation is in the arch_topology "driver" (GENERIC_ARCH_TOPOLOGY). Then, the caller of arch_set_thermal_pressure() is cpufreq_cooling (see below); that'll only get called if you have thermal zones using CPU cooling devices. AFAICT the current state of things imply we should have something like depends on (ARM || ARM64) && GENERIC_ARCH_TOPOLOGY for that option. >> > > + help >> > > + This option allows the scheduler to be aware of CPU thermal throttling >> > > + (i.e. thermal pressure), providing arch_scale_thermal_pressure() is >> > > + implemented. > > Is this feature documented in terms of what it does? Do I assume that > as the thermal trip points start tripping, that has an influence on > the scheduler? Or is it the case that the scheduler is wanting to > know when the cpu frequency changes? > > Grepping for "thermal" in Documentation/scheduler brings up nothing. The former; changing a CPU cooling device's state (IOW changing its max allowed frequency for thermal reasons) leads to a call to arch_set_thermal_pressure() (see cpufreq_cooling.c::cpufreq_set_cur_state()). It's somewhat interesting to have, at least in theory. On plain SMP that would let the scheduler see if some CPUs are more throttled that others, which would be leveraged when doing load balancing. It's more interesting for big.LITTLE & co, where in the worst cases we can have things like capacity inversion, i.e. the bigs are so thermally throttled that they give less oomf than a LITTLE. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-03 20:25 ` Valentin Schneider @ 2020-06-04 0:48 ` Thara Gopinath 2020-06-04 9:26 ` Valentin Schneider 0 siblings, 1 reply; 17+ messages in thread From: Thara Gopinath @ 2020-06-04 0:48 UTC (permalink / raw) To: Valentin Schneider, Russell King - ARM Linux admin Cc: Vincent Guittot, linux-kernel On 6/3/20 4:25 PM, Valentin Schneider wrote: > > On 03/06/20 20:58, Russell King - ARM Linux admin wrote: >> On Wed, Jun 03, 2020 at 09:24:56PM +0200, Vincent Guittot wrote: >>> On Wed, 3 Jun 2020 at 20:45, Russell King - ARM Linux admin >>> <linux@armlinux.org.uk> wrote: >>>> It's a start. I'm still wondering whether I should answer yes or no >>>> for the platforms I'm building for. >>>> >>>> So far, all I've found is: >>>> >>>> arch/arm/include/asm/topology.h:#define arch_scale_thermal_pressure topology_get_thermal_pressure >>>> >>>> which really doesn't tell me anything about this. So I'm still in >>>> the dark. >>>> >>>> I guess topology_get_thermal_pressure is provided by something in >>>> drivers/ which will be conditional on some driver or something. >>> >>> You need cpufreq_cooling device to make it useful and only for SMP >>> I don't think that this should not be user configurable because even >>> with the description above, it is not easy to choose. >>> This should be set by the driver that implement the feature which is >>> only cpufreq cooling device for now it >> >> As I have CONFIG_CPU_FREQ_THERMAL=y in my config, I'm guessing (and it's >> only a guess) that I should say y to SCHED_THERMAL_PRESSURE ? >> > > arm and arm64 implement arch_scale_thermal_pressure(); the actual > implementation is in the arch_topology "driver" (GENERIC_ARCH_TOPOLOGY). > > Then, the caller of arch_set_thermal_pressure() is cpufreq_cooling (see > below); that'll only get called if you have thermal zones using CPU > cooling devices. > > AFAICT the current state of things imply we should have something like > > depends on (ARM || ARM64) && GENERIC_ARCH_TOPOLOGY > > for that option. Hi Russel/Valentin The feature itself like Valentin explained below allows scheduler to be aware of cpu capacity reduced due to thermal throttling. arch_set_thermal_pressure feeds the capped capacity to the scheduler and hence the feature makes sense only if arch_set_thermal_pressure is implemented. Having said that arch_set_thermal_pressure is implemented in arch_topology driver for arm and arm64 platforms. But the feature itself is not bound to arm/arm64 platforms. So it would make it wrong to add a "depends on (ARM || ARM64) option." I agree with Vincent that allowing user to choose this option is probably not the best. IMO, this should be enabled by default in arm64 defconfig considering both GENERIC_ARCH_TOPOLOGY and CPU_FREQ_THERMAL are enabled by default. So if it is acceptable three things to be done are: 1. Add the help text. 2. Don't allow SCHED_THERMAL_PRESSURE configurable by user 3. Enable it by default in arm64 defconfig > >>>>> + help >>>>> + This option allows the scheduler to be aware of CPU thermal throttling >>>>> + (i.e. thermal pressure), providing arch_scale_thermal_pressure() is >>>>> + implemented. >> >> Is this feature documented in terms of what it does? Do I assume that >> as the thermal trip points start tripping, that has an influence on >> the scheduler? Or is it the case that the scheduler is wanting to >> know when the cpu frequency changes? >> >> Grepping for "thermal" in Documentation/scheduler brings up nothing. > > The former; changing a CPU cooling device's state (IOW changing its max > allowed frequency for thermal reasons) leads to a call to > arch_set_thermal_pressure() (see > cpufreq_cooling.c::cpufreq_set_cur_state()). > > It's somewhat interesting to have, at least in theory. On plain SMP that > would let the scheduler see if some CPUs are more throttled that others, > which would be leveraged when doing load balancing. It's more > interesting for big.LITTLE & co, where in the worst cases we can have > things like capacity inversion, i.e. the bigs are so thermally throttled > that they give less oomf than a LITTLE. > -- Warm Regards Thara ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-04 0:48 ` Thara Gopinath @ 2020-06-04 9:26 ` Valentin Schneider 2020-06-04 9:29 ` Russell King - ARM Linux admin 0 siblings, 1 reply; 17+ messages in thread From: Valentin Schneider @ 2020-06-04 9:26 UTC (permalink / raw) To: Thara Gopinath Cc: Russell King - ARM Linux admin, Vincent Guittot, linux-kernel On 04/06/20 01:48, Thara Gopinath wrote: > Hi Russel/Valentin > > The feature itself like Valentin explained below allows scheduler to be > aware of cpu capacity reduced due to thermal throttling. > arch_set_thermal_pressure feeds the capped capacity to the scheduler and > hence the feature makes sense only if arch_set_thermal_pressure is > implemented. Having said that arch_set_thermal_pressure is implemented > in arch_topology driver for arm and arm64 platforms. But the feature > itself is not bound to arm/arm64 platforms. So it would make it wrong to > add a "depends on (ARM || ARM64) option." > > I agree with Vincent that allowing user to choose this option is > probably not the best. IMO, this should be enabled by default in arm64 > defconfig considering both GENERIC_ARCH_TOPOLOGY and CPU_FREQ_THERMAL > are enabled by default. Right, I had skimmed over that but it probably does make more sense not to bother users with it. > So if it is acceptable three things to be done are: > 1. Add the help text. > 2. Don't allow SCHED_THERMAL_PRESSURE configurable by user > 3. Enable it by default in arm64 defconfig ... and arm as well, I suppose? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-04 9:26 ` Valentin Schneider @ 2020-06-04 9:29 ` Russell King - ARM Linux admin 2020-06-04 10:56 ` Valentin Schneider 0 siblings, 1 reply; 17+ messages in thread From: Russell King - ARM Linux admin @ 2020-06-04 9:29 UTC (permalink / raw) To: Valentin Schneider; +Cc: Thara Gopinath, Vincent Guittot, linux-kernel On Thu, Jun 04, 2020 at 10:26:27AM +0100, Valentin Schneider wrote: > > On 04/06/20 01:48, Thara Gopinath wrote: > > Hi Russel/Valentin > > > > The feature itself like Valentin explained below allows scheduler to be > > aware of cpu capacity reduced due to thermal throttling. > > arch_set_thermal_pressure feeds the capped capacity to the scheduler and > > hence the feature makes sense only if arch_set_thermal_pressure is > > implemented. Having said that arch_set_thermal_pressure is implemented > > in arch_topology driver for arm and arm64 platforms. But the feature > > itself is not bound to arm/arm64 platforms. So it would make it wrong to > > add a "depends on (ARM || ARM64) option." > > > > I agree with Vincent that allowing user to choose this option is > > probably not the best. IMO, this should be enabled by default in arm64 > > defconfig considering both GENERIC_ARCH_TOPOLOGY and CPU_FREQ_THERMAL > > are enabled by default. > > Right, I had skimmed over that but it probably does make more sense not > to bother users with it. > > > So if it is acceptable three things to be done are: > > 1. Add the help text. > > 2. Don't allow SCHED_THERMAL_PRESSURE configurable by user > > 3. Enable it by default in arm64 defconfig > > ... and arm as well, I suppose? If it's not a user visible option, then there's no point it being in defconfig. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-04 9:29 ` Russell King - ARM Linux admin @ 2020-06-04 10:56 ` Valentin Schneider [not found] ` <CALD-y_zQms4YQup2MgAfNhWSu=ewkhossHma2TKqfTcOFaG=uA@mail.gmail.com> 0 siblings, 1 reply; 17+ messages in thread From: Valentin Schneider @ 2020-06-04 10:56 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Thara Gopinath, Vincent Guittot, linux-kernel On 04/06/20 10:29, Russell King - ARM Linux admin wrote: > On Thu, Jun 04, 2020 at 10:26:27AM +0100, Valentin Schneider wrote: >> >> On 04/06/20 01:48, Thara Gopinath wrote: >> > Hi Russel/Valentin >> > >> > The feature itself like Valentin explained below allows scheduler to be >> > aware of cpu capacity reduced due to thermal throttling. >> > arch_set_thermal_pressure feeds the capped capacity to the scheduler and >> > hence the feature makes sense only if arch_set_thermal_pressure is >> > implemented. Having said that arch_set_thermal_pressure is implemented >> > in arch_topology driver for arm and arm64 platforms. But the feature >> > itself is not bound to arm/arm64 platforms. So it would make it wrong to >> > add a "depends on (ARM || ARM64) option." >> > >> > I agree with Vincent that allowing user to choose this option is >> > probably not the best. IMO, this should be enabled by default in arm64 >> > defconfig considering both GENERIC_ARCH_TOPOLOGY and CPU_FREQ_THERMAL >> > are enabled by default. >> >> Right, I had skimmed over that but it probably does make more sense not >> to bother users with it. >> >> > So if it is acceptable three things to be done are: >> > 1. Add the help text. >> > 2. Don't allow SCHED_THERMAL_PRESSURE configurable by user >> > 3. Enable it by default in arm64 defconfig >> >> ... and arm as well, I suppose? > > If it's not a user visible option, then there's no point it being in > defconfig. Right, s/defconfig/arch kconfig/ or somesuch. ^ permalink raw reply [flat|nested] 17+ messages in thread
[parent not found: <CALD-y_zQms4YQup2MgAfNhWSu=ewkhossHma2TKqfTcOFaG=uA@mail.gmail.com>]
* Re: v5.7: new core kernel option missing help text [not found] ` <CALD-y_zQms4YQup2MgAfNhWSu=ewkhossHma2TKqfTcOFaG=uA@mail.gmail.com> @ 2020-06-04 15:38 ` Valentin Schneider 2020-06-04 22:22 ` Thara Gopinath ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Valentin Schneider @ 2020-06-04 15:38 UTC (permalink / raw) To: Thara Gopinath Cc: Russell King - ARM Linux admin, Vincent Guittot, linux-kernel On 04/06/20 14:05, Thara Gopinath wrote: > On Thu, 4 Jun 2020 at 06:56, Valentin Schneider <valentin.schneider@arm.com> >> >> Right, s/defconfig/arch kconfig/ or somesuch. >> > > CPU_FREQ_THERMAL also has to be enabled for this to be effective. > Since arm64 defconfig enables CPU_FREQ_THERMAL (by enabling CPU_THERMAL), > it should be ok to enable it in arm64/Kconfig. (same with arm/Kconfig) > > Another option is to select the SCHED_THERMAL_PRESSURE when > CPU_FREQ_THERMAL > is enabled in drivers/thermal/Kconfig. > So interestingly while arch_set_thermal_pressure() (which just writes to a pcpu variable) is defined in sched/core.c, arch_scale_thermal_pressure() (which just returns aforementionned pcpu variable) is defined in arch_topology... I'm thinking at this point we might as well turn the arch_scale_thermal_pressure() stub into what arch_topology does. This would effectively let any architecture use thermal pressure, providing they use cpufreq cooling. If we want to keep changes contained to Kconfigs, for now I think the safest would be: --- diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 16fbf74030fe..1e92080dc275 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -46,6 +46,7 @@ config ARM select EDAC_ATOMIC_SCRUB select GENERIC_ALLOCATOR select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY + select SCHED_THERMAL_PRESSURE if GENERIC_ARCH_TOPOLOGY select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI select GENERIC_CLOCKEVENTS_BROADCAST if SMP select GENERIC_CPU_AUTOPROBE diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 552d36cacc05..cc1944fbae51 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -98,6 +98,7 @@ config ARM64 select FRAME_POINTER select GENERIC_ALLOCATOR select GENERIC_ARCH_TOPOLOGY + select SCHED_THERMAL_PRESSURE select GENERIC_CLOCKEVENTS select GENERIC_CLOCKEVENTS_BROADCAST select GENERIC_CPU_AUTOPROBE diff --git a/init/Kconfig b/init/Kconfig index 74a5ac65644f..ba846f6e805b 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -439,8 +439,11 @@ config HAVE_SCHED_AVG_IRQ depends on SMP config SCHED_THERMAL_PRESSURE - bool "Enable periodic averaging of thermal pressure" + def_bool n depends on SMP + depends on CPU_FREQ_THERMAL + help + <helpful thing here> config BSD_PROCESS_ACCT bool "BSD Process Accounting" --- > Warm Regards > Thara ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-04 15:38 ` Valentin Schneider @ 2020-06-04 22:22 ` Thara Gopinath 2020-06-05 7:03 ` Vincent Guittot 2020-06-05 8:15 ` Russell King - ARM Linux admin 2 siblings, 0 replies; 17+ messages in thread From: Thara Gopinath @ 2020-06-04 22:22 UTC (permalink / raw) To: Valentin Schneider Cc: Russell King - ARM Linux admin, Vincent Guittot, linux-kernel On 6/4/20 11:38 AM, Valentin Schneider wrote: > > On 04/06/20 14:05, Thara Gopinath wrote: >> On Thu, 4 Jun 2020 at 06:56, Valentin Schneider <valentin.schneider@arm.com> >>> >>> Right, s/defconfig/arch kconfig/ or somesuch. >>> >> >> CPU_FREQ_THERMAL also has to be enabled for this to be effective. >> Since arm64 defconfig enables CPU_FREQ_THERMAL (by enabling CPU_THERMAL), >> it should be ok to enable it in arm64/Kconfig. (same with arm/Kconfig) >> >> Another option is to select the SCHED_THERMAL_PRESSURE when >> CPU_FREQ_THERMAL >> is enabled in drivers/thermal/Kconfig. >> > > So interestingly while arch_set_thermal_pressure() (which just writes to a > pcpu variable) is defined in sched/core.c, arch_scale_thermal_pressure() > (which just returns aforementionned pcpu variable) is defined in > arch_topology... > > I'm thinking at this point we might as well turn the > arch_scale_thermal_pressure() stub into what arch_topology does. This would > effectively let any architecture use thermal pressure, providing they use > cpufreq cooling. > > If we want to keep changes contained to Kconfigs, for now I think the > safest would be: Thanks Valentin. This looks good to me for now. Although, I want to state that the thermal pressure can be set by some other entity other than cpufreq cooling as well. But currently only cpufreq cooling handles it. So for now the below looks good. > > --- > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 16fbf74030fe..1e92080dc275 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -46,6 +46,7 @@ config ARM > select EDAC_ATOMIC_SCRUB > select GENERIC_ALLOCATOR > select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY > + select SCHED_THERMAL_PRESSURE if GENERIC_ARCH_TOPOLOGY > select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI > select GENERIC_CLOCKEVENTS_BROADCAST if SMP > select GENERIC_CPU_AUTOPROBE > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 552d36cacc05..cc1944fbae51 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -98,6 +98,7 @@ config ARM64 > select FRAME_POINTER > select GENERIC_ALLOCATOR > select GENERIC_ARCH_TOPOLOGY > + select SCHED_THERMAL_PRESSURE > select GENERIC_CLOCKEVENTS > select GENERIC_CLOCKEVENTS_BROADCAST > select GENERIC_CPU_AUTOPROBE > diff --git a/init/Kconfig b/init/Kconfig > index 74a5ac65644f..ba846f6e805b 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -439,8 +439,11 @@ config HAVE_SCHED_AVG_IRQ > depends on SMP > > config SCHED_THERMAL_PRESSURE > - bool "Enable periodic averaging of thermal pressure" > + def_bool n > depends on SMP > + depends on CPU_FREQ_THERMAL > + help > + <helpful thing here> > > config BSD_PROCESS_ACCT > bool "BSD Process Accounting" > --- > > > >> Warm Regards >> Thara -- Warm Regards Thara ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-04 15:38 ` Valentin Schneider 2020-06-04 22:22 ` Thara Gopinath @ 2020-06-05 7:03 ` Vincent Guittot 2020-06-05 9:46 ` Valentin Schneider 2020-06-05 8:15 ` Russell King - ARM Linux admin 2 siblings, 1 reply; 17+ messages in thread From: Vincent Guittot @ 2020-06-05 7:03 UTC (permalink / raw) To: Valentin Schneider Cc: Thara Gopinath, Russell King - ARM Linux admin, linux-kernel On Thu, 4 Jun 2020 at 17:38, Valentin Schneider <valentin.schneider@arm.com> wrote: > > > On 04/06/20 14:05, Thara Gopinath wrote: > > On Thu, 4 Jun 2020 at 06:56, Valentin Schneider <valentin.schneider@arm.com> > >> > >> Right, s/defconfig/arch kconfig/ or somesuch. > >> > > > > CPU_FREQ_THERMAL also has to be enabled for this to be effective. > > Since arm64 defconfig enables CPU_FREQ_THERMAL (by enabling CPU_THERMAL), > > it should be ok to enable it in arm64/Kconfig. (same with arm/Kconfig) > > > > Another option is to select the SCHED_THERMAL_PRESSURE when > > CPU_FREQ_THERMAL > > is enabled in drivers/thermal/Kconfig. > > > > So interestingly while arch_set_thermal_pressure() (which just writes to a > pcpu variable) is defined in sched/core.c, arch_scale_thermal_pressure() > (which just returns aforementionned pcpu variable) is defined in > arch_topology... > > I'm thinking at this point we might as well turn the > arch_scale_thermal_pressure() stub into what arch_topology does. This would > effectively let any architecture use thermal pressure, providing they use > cpufreq cooling. > > If we want to keep changes contained to Kconfigs, for now I think the > safest would be: > > --- > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 16fbf74030fe..1e92080dc275 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -46,6 +46,7 @@ config ARM > select EDAC_ATOMIC_SCRUB > select GENERIC_ALLOCATOR > select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY > + select SCHED_THERMAL_PRESSURE if GENERIC_ARCH_TOPOLOGY I think that SCHED_THERMAL_PRESSURE depends on ARM_CPU_TOPOLOGY but not on GENERIC_ARCH_TOPOLOGY. ARM_CPU_TOPOLOGY is used to define arch_scale_thermal_pressure for arm architecture and we only use the header file of the generic arch_topology.h. Function like arch_set_thermal_pressure() is in sched/core.c > select GENERIC_ATOMIC64 if CPU_V7M || CPU_V6 || !CPU_32v6K || !AEABI > select GENERIC_CLOCKEVENTS_BROADCAST if SMP > select GENERIC_CPU_AUTOPROBE > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 552d36cacc05..cc1944fbae51 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -98,6 +98,7 @@ config ARM64 > select FRAME_POINTER > select GENERIC_ALLOCATOR > select GENERIC_ARCH_TOPOLOGY > + select SCHED_THERMAL_PRESSURE > select GENERIC_CLOCKEVENTS > select GENERIC_CLOCKEVENTS_BROADCAST > select GENERIC_CPU_AUTOPROBE > diff --git a/init/Kconfig b/init/Kconfig > index 74a5ac65644f..ba846f6e805b 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -439,8 +439,11 @@ config HAVE_SCHED_AVG_IRQ > depends on SMP > > config SCHED_THERMAL_PRESSURE > - bool "Enable periodic averaging of thermal pressure" > + def_bool n > depends on SMP > + depends on CPU_FREQ_THERMAL > + help > + <helpful thing here> > > config BSD_PROCESS_ACCT > bool "BSD Process Accounting" > --- > > > > > Warm Regards > > Thara ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-05 7:03 ` Vincent Guittot @ 2020-06-05 9:46 ` Valentin Schneider 0 siblings, 0 replies; 17+ messages in thread From: Valentin Schneider @ 2020-06-05 9:46 UTC (permalink / raw) To: Vincent Guittot Cc: Thara Gopinath, Russell King - ARM Linux admin, linux-kernel On 05/06/20 08:03, Vincent Guittot wrote: > On Thu, 4 Jun 2020 at 17:38, Valentin Schneider > <valentin.schneider@arm.com> wrote: >> --- >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig >> index 16fbf74030fe..1e92080dc275 100644 >> --- a/arch/arm/Kconfig >> +++ b/arch/arm/Kconfig >> @@ -46,6 +46,7 @@ config ARM >> select EDAC_ATOMIC_SCRUB >> select GENERIC_ALLOCATOR >> select GENERIC_ARCH_TOPOLOGY if ARM_CPU_TOPOLOGY >> + select SCHED_THERMAL_PRESSURE if GENERIC_ARCH_TOPOLOGY > > I think that SCHED_THERMAL_PRESSURE depends on ARM_CPU_TOPOLOGY but > not on GENERIC_ARCH_TOPOLOGY. > ARM_CPU_TOPOLOGY is used to define arch_scale_thermal_pressure for arm > architecture > and we only use the header file of the generic arch_topology.h. > Function like arch_set_thermal_pressure() is in sched/core.c > You're right, oh well... Let me spend a bit more time on this and I'll send actual patches. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-04 15:38 ` Valentin Schneider 2020-06-04 22:22 ` Thara Gopinath 2020-06-05 7:03 ` Vincent Guittot @ 2020-06-05 8:15 ` Russell King - ARM Linux admin 2020-06-05 10:02 ` Valentin Schneider 2 siblings, 1 reply; 17+ messages in thread From: Russell King - ARM Linux admin @ 2020-06-05 8:15 UTC (permalink / raw) To: Valentin Schneider; +Cc: Thara Gopinath, Vincent Guittot, linux-kernel On Thu, Jun 04, 2020 at 04:38:40PM +0100, Valentin Schneider wrote: > diff --git a/init/Kconfig b/init/Kconfig > index 74a5ac65644f..ba846f6e805b 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -439,8 +439,11 @@ config HAVE_SCHED_AVG_IRQ > depends on SMP > > config SCHED_THERMAL_PRESSURE > - bool "Enable periodic averaging of thermal pressure" > + def_bool n You don't need to specify this default, as the default default is "n". "bool" will do. You should never need to use "def_bool n" or "default n" for anything this simple. There are more complex cases where there may be multiple conditional "default" lines where it may be necessary, but the majority of cases where people use these are completely unnecessary. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC for 0.8m (est. 1762m) line in suburbia: sync at 13.1Mbps down 424kbps up ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: v5.7: new core kernel option missing help text 2020-06-05 8:15 ` Russell King - ARM Linux admin @ 2020-06-05 10:02 ` Valentin Schneider 0 siblings, 0 replies; 17+ messages in thread From: Valentin Schneider @ 2020-06-05 10:02 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: Thara Gopinath, Vincent Guittot, linux-kernel On 05/06/20 09:15, Russell King - ARM Linux admin wrote: > On Thu, Jun 04, 2020 at 04:38:40PM +0100, Valentin Schneider wrote: >> diff --git a/init/Kconfig b/init/Kconfig >> index 74a5ac65644f..ba846f6e805b 100644 >> --- a/init/Kconfig >> +++ b/init/Kconfig >> @@ -439,8 +439,11 @@ config HAVE_SCHED_AVG_IRQ >> depends on SMP >> >> config SCHED_THERMAL_PRESSURE >> - bool "Enable periodic averaging of thermal pressure" >> + def_bool n > > You don't need to specify this default, as the default default is "n". > "bool" will do. > > You should never need to use "def_bool n" or "default n" for anything > this simple. There are more complex cases where there may be multiple > conditional "default" lines where it may be necessary, but the majority > of cases where people use these are completely unnecessary. Right, thanks! ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-06-05 10:02 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 17:31 v5.7: new core kernel option missing help text Russell King - ARM Linux admin
2020-06-03 18:00 ` Valentin Schneider
2020-06-03 18:45 ` Russell King - ARM Linux admin
2020-06-03 19:24 ` Vincent Guittot
2020-06-03 19:58 ` Russell King - ARM Linux admin
2020-06-03 20:22 ` Vincent Guittot
2020-06-03 20:25 ` Valentin Schneider
2020-06-04 0:48 ` Thara Gopinath
2020-06-04 9:26 ` Valentin Schneider
2020-06-04 9:29 ` Russell King - ARM Linux admin
2020-06-04 10:56 ` Valentin Schneider
[not found] ` <CALD-y_zQms4YQup2MgAfNhWSu=ewkhossHma2TKqfTcOFaG=uA@mail.gmail.com>
2020-06-04 15:38 ` Valentin Schneider
2020-06-04 22:22 ` Thara Gopinath
2020-06-05 7:03 ` Vincent Guittot
2020-06-05 9:46 ` Valentin Schneider
2020-06-05 8:15 ` Russell King - ARM Linux admin
2020-06-05 10:02 ` Valentin Schneider
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).