| Message ID | 1348827774-17880-1-git-send-email-fwu@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 28 Sep 2012, Fan Wu wrote: > From: fwu <fwu@marvell.com> > > 1. On ARM platform, "nohlt" can be used to prevent core from idle > process, returning immediately. > 2. There are two interfaces, exported for other modules, named > "disable_hlt" and "enable_hlt" are used to enable/disable the > cpuidle mechanism by increasing/decreasing "hlt_counter". > Disable_hlt and enable_hlt are paired operation, > when you first call disable_hlt and then enable_hlt, the > semantics are right. > 3. There is no obvious constraint to prevent user(driver/module) > code to prevent the case that enable_hlt is ahead of disable_hlt, > which is a fatal operation on kernel state change from user, > and there is no any WARNING or notification if the case happens > in current kernel code. > This patch aims to report BUG when the case happens, just like > what the kernel do when enable_irq is ahead of disable_irq. > > Signed-off-by: fwu <fwu@marvell.com> > Signed-off-by: YiLu Mao <ylmao@marvell.com> > Signed-off-by: Ning Jiang <ning.jiang@marvell.com> This looks sensible to me. Acked-by: Nicolas Pitre <nico@linaro.org> > Change-Id: Iddacf45aec012bb663e130801727b4eb3d11436b Please don't forget to strip that when submitting your patch to Russell's patch system. > --- > arch/arm/kernel/process.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c > index 693b744..5bbd3be 100644 > --- a/arch/arm/kernel/process.c > +++ b/arch/arm/kernel/process.c > @@ -70,6 +70,7 @@ EXPORT_SYMBOL(disable_hlt); > void enable_hlt(void) > { > hlt_counter--; > + BUG_ON(hlt_counter < 0) > } > > EXPORT_SYMBOL(enable_hlt); > -- > 1.7.0.4 >
On 09/29/2012 03:45 AM, Nicolas Pitre wrote: > On Fri, 28 Sep 2012, Fan Wu wrote: > >> From: fwu <fwu@marvell.com> >> >> 1. On ARM platform, "nohlt" can be used to prevent core from idle >> process, returning immediately. >> 2. There are two interfaces, exported for other modules, named >> "disable_hlt" and "enable_hlt" are used to enable/disable the >> cpuidle mechanism by increasing/decreasing "hlt_counter". >> Disable_hlt and enable_hlt are paired operation, >> when you first call disable_hlt and then enable_hlt, the >> semantics are right. >> 3. There is no obvious constraint to prevent user(driver/module) >> code to prevent the case that enable_hlt is ahead of disable_hlt, >> which is a fatal operation on kernel state change from user, >> and there is no any WARNING or notification if the case happens >> in current kernel code. >> This patch aims to report BUG when the case happens, just like >> what the kernel do when enable_irq is ahead of disable_irq. >> >> Signed-off-by: fwu <fwu@marvell.com> >> Signed-off-by: YiLu Mao <ylmao@marvell.com> >> Signed-off-by: Ning Jiang <ning.jiang@marvell.com> > This looks sensible to me. > > Acked-by: Nicolas Pitre <nico@linaro.org> > >> Change-Id: Iddacf45aec012bb663e130801727b4eb3d11436b > Please don't forget to strip that when submitting your patch to > Russell's patch system. > >> --- >> arch/arm/kernel/process.c | 1 + >> 1 files changed, 1 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c >> index 693b744..5bbd3be 100644 >> --- a/arch/arm/kernel/process.c >> +++ b/arch/arm/kernel/process.c >> @@ -70,6 +70,7 @@ EXPORT_SYMBOL(disable_hlt); >> void enable_hlt(void) >> { >> hlt_counter--; >> + BUG_ON(hlt_counter < 0) >> } >> >> EXPORT_SYMBOL(enable_hlt); >> -- >> 1.7.0.4 >> Thanks a lot for reviewing the patch. I'll remove that in the new patch version. Thanks again.
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index 693b744..5bbd3be 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -70,6 +70,7 @@ EXPORT_SYMBOL(disable_hlt); void enable_hlt(void) { hlt_counter--; + BUG_ON(hlt_counter < 0) } EXPORT_SYMBOL(enable_hlt);