| Message ID | 48afad7788300482c047fc35e70ca8e4bf31a5ac.1471557381.git.ego@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On 08/19/2016 12:26 AM, Gautham R. Shenoy wrote: > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> > > Currently all the idle states registered by a cpu-idle driver are > enabled by default. This patch adds a mechanism which allows the > driver to hint if an idle-state should start in a disabled state. The > cpu-idle core will use this hint to appropriately initialize the > usage->disable knob of the CPU device idle state. Why do you need to do that ? > The state can be enabled at run time by echo'ing a zero to the sysfs > "disable" control file. ... for each cpu.
On 25/08/16 00:44, Daniel Lezcano wrote: > On 08/19/2016 12:26 AM, Gautham R. Shenoy wrote: >> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> >> >> Currently all the idle states registered by a cpu-idle driver are >> enabled by default. This patch adds a mechanism which allows the >> driver to hint if an idle-state should start in a disabled state. The >> cpu-idle core will use this hint to appropriately initialize the >> usage->disable knob of the CPU device idle state. > > Why do you need to do that ? > I think patch 2/2 explains the reason as it uses this infrastructure Balbir Singh -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/24/2016 04:48 PM, Balbir Singh wrote: > > > On 25/08/16 00:44, Daniel Lezcano wrote: >> On 08/19/2016 12:26 AM, Gautham R. Shenoy wrote: >>> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> >>> >>> Currently all the idle states registered by a cpu-idle driver are >>> enabled by default. This patch adds a mechanism which allows the >>> driver to hint if an idle-state should start in a disabled state. The >>> cpu-idle core will use this hint to appropriately initialize the >>> usage->disable knob of the CPU device idle state. >> >> Why do you need to do that ? >> > > I think patch 2/2 explains the reason as it uses this infrastructure Ok, let me elaborate the question, I was not clear. Why the userspace can't setup the system environment at boot time by disabling the state instead of adding extra code to disable it at boot time in the kernel and then re-enable it from userspace ? -- Daniel
On 25/08/16 01:06, Daniel Lezcano wrote: > On 08/24/2016 04:48 PM, Balbir Singh wrote: >> >> >> On 25/08/16 00:44, Daniel Lezcano wrote: >>> On 08/19/2016 12:26 AM, Gautham R. Shenoy wrote: >>>> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> >>>> >>>> Currently all the idle states registered by a cpu-idle driver are >>>> enabled by default. This patch adds a mechanism which allows the >>>> driver to hint if an idle-state should start in a disabled state. The >>>> cpu-idle core will use this hint to appropriately initialize the >>>> usage->disable knob of the CPU device idle state. >>> >>> Why do you need to do that ? >>> >> >> I think patch 2/2 explains the reason as it uses this infrastructure > > Ok, let me elaborate the question, I was not clear. > > Why the userspace can't setup the system environment at boot time by > disabling the state instead of adding extra code to disable it at boot > time in the kernel and then re-enable it from userspace ? Gautham's patches don't want to have those states enabled by default. They are unlikely to be what production systems need, but likely what a knowledgeable person can look into selectively enable for experimentation. @Gautham? Balbir Singh. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/25/2016 03:46 PM, Balbir Singh wrote: > > > On 25/08/16 01:06, Daniel Lezcano wrote: >> On 08/24/2016 04:48 PM, Balbir Singh wrote: >>> >>> >>> On 25/08/16 00:44, Daniel Lezcano wrote: >>>> On 08/19/2016 12:26 AM, Gautham R. Shenoy wrote: >>>>> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> >>>>> >>>>> Currently all the idle states registered by a cpu-idle driver are >>>>> enabled by default. This patch adds a mechanism which allows the >>>>> driver to hint if an idle-state should start in a disabled state. The >>>>> cpu-idle core will use this hint to appropriately initialize the >>>>> usage->disable knob of the CPU device idle state. >>>> >>>> Why do you need to do that ? >>>> >>> >>> I think patch 2/2 explains the reason as it uses this infrastructure >> >> Ok, let me elaborate the question, I was not clear. >> >> Why the userspace can't setup the system environment at boot time by >> disabling the state instead of adding extra code to disable it at boot >> time in the kernel and then re-enable it from userspace ? > > Gautham's patches don't want to have those states enabled by default. > They are unlikely to be what production systems need, but likely > what a knowledgeable person can look into selectively enable for > experimentation. Why not invert the logic ? A knowledgeable person can look into selectively disable for production. In addition, a kernel command line option to specify which state to disable would be appropriate and beneficial for all existing drivers.
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index c73207a..b4debc7 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -439,7 +439,14 @@ static void __cpuidle_unregister_device(struct cpuidle_device *dev) static void __cpuidle_device_init(struct cpuidle_device *dev) { + struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev); + int i; + memset(dev->states_usage, 0, sizeof(dev->states_usage)); + for (i = 0; i < drv->state_count; i++) { + if (drv->states[i].disable_use_at_start) + dev->states_usage[i].disable = 1; + } dev->last_residency = 0; } diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h index bb31373..f3fe855 100644 --- a/include/linux/cpuidle.h +++ b/include/linux/cpuidle.h @@ -44,7 +44,12 @@ struct cpuidle_state { int power_usage; /* in mW */ unsigned int target_residency; /* in US */ bool disabled; /* disabled on all CPUs */ - + /* + * disable_use_at_start: If true, then this idle state will be + * disabled by default. It can be enabled at runtime using the + * per-cpu cpuidle sysfs control file named "disable". + */ + bool disable_use_at_start; int (*enter) (struct cpuidle_device *dev, struct cpuidle_driver *drv, int index);