linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: Add BUG_ON when hlt counter is wrongly used
@ 2012-09-28 10:22 Fan Wu
  2012-09-28 19:45 ` Nicolas Pitre
  0 siblings, 1 reply; 5+ messages in thread
From: Fan Wu @ 2012-09-28 10:22 UTC (permalink / raw)
  To: linux-arm-kernel

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>

Change-Id: Iddacf45aec012bb663e130801727b4eb3d11436b
---
 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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] ARM: Add BUG_ON when hlt counter is wrongly used
@ 2012-09-28 10:59 Fan Wu
  0 siblings, 0 replies; 5+ messages in thread
From: Fan Wu @ 2012-09-28 10:59 UTC (permalink / raw)
  To: linux-arm-kernel

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>

Change-Id: Iddacf45aec012bb663e130801727b4eb3d11436b
---
 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..d3f2972 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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] ARM: Add BUG_ON when hlt counter is wrongly used
  2012-09-28 10:22 Fan Wu
@ 2012-09-28 19:45 ` Nicolas Pitre
  2012-09-29  0:45   ` FanWu
  0 siblings, 1 reply; 5+ messages in thread
From: Nicolas Pitre @ 2012-09-28 19:45 UTC (permalink / raw)
  To: linux-arm-kernel

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
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] ARM: Add BUG_ON when hlt counter is wrongly used
  2012-09-28 19:45 ` Nicolas Pitre
@ 2012-09-29  0:45   ` FanWu
  0 siblings, 0 replies; 5+ messages in thread
From: FanWu @ 2012-09-29  0:45 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] ARM: Add BUG_ON when hlt counter is wrongly used
@ 2012-09-29  0:48 Fan Wu
  0 siblings, 0 replies; 5+ messages in thread
From: Fan Wu @ 2012-09-29  0:48 UTC (permalink / raw)
  To: linux-arm-kernel

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>

---
 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..d3f2972 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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-09-29  0:48 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-28 10:59 [PATCH] ARM: Add BUG_ON when hlt counter is wrongly used Fan Wu
  -- strict thread matches above, loose matches on Subject: below --
2012-09-29  0:48 Fan Wu
2012-09-28 10:22 Fan Wu
2012-09-28 19:45 ` Nicolas Pitre
2012-09-29  0:45   ` FanWu

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).