Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-08-15 Thread Rafael J. Wysocki
On Wednesday, August 08, 2012, Colin Cross wrote:
> On Tue, Aug 7, 2012 at 3:15 PM, Rafael J. Wysocki  wrote:
> > On Wednesday, July 25, 2012, Colin Cross wrote:
> >> The cpu hotplug notifier gets called in both atomic and non-atomic
> >> contexts, it is not always safe to lock a mutex.  Filter out all events
> >> except the six necessary ones, which are all sleepable, before taking
> >> the mutex.
> >>
> >> Signed-off-by: Colin Cross 
> >
> > Has this been applied already?
> 
> It's not in Linus' tree, and I haven't heard anything from Len.

Len appears to be unavailable.

I'll put it into the linux-next branch of the linux-pm.git tree and will
push it to Linus for -rc3 if Len doesn't show up till then.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-08-15 Thread Rafael J. Wysocki
On Wednesday, August 08, 2012, Colin Cross wrote:
 On Tue, Aug 7, 2012 at 3:15 PM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Wednesday, July 25, 2012, Colin Cross wrote:
  The cpu hotplug notifier gets called in both atomic and non-atomic
  contexts, it is not always safe to lock a mutex.  Filter out all events
  except the six necessary ones, which are all sleepable, before taking
  the mutex.
 
  Signed-off-by: Colin Cross ccr...@android.com
 
  Has this been applied already?
 
 It's not in Linus' tree, and I haven't heard anything from Len.

Len appears to be unavailable.

I'll put it into the linux-next branch of the linux-pm.git tree and will
push it to Linus for -rc3 if Len doesn't show up till then.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-08-07 Thread Colin Cross
On Tue, Aug 7, 2012 at 3:15 PM, Rafael J. Wysocki  wrote:
> On Wednesday, July 25, 2012, Colin Cross wrote:
>> The cpu hotplug notifier gets called in both atomic and non-atomic
>> contexts, it is not always safe to lock a mutex.  Filter out all events
>> except the six necessary ones, which are all sleepable, before taking
>> the mutex.
>>
>> Signed-off-by: Colin Cross 
>
> Has this been applied already?

It's not in Linus' tree, and I haven't heard anything from Len.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-08-07 Thread Rafael J. Wysocki
On Wednesday, July 25, 2012, Colin Cross wrote:
> The cpu hotplug notifier gets called in both atomic and non-atomic
> contexts, it is not always safe to lock a mutex.  Filter out all events
> except the six necessary ones, which are all sleepable, before taking
> the mutex.
> 
> Signed-off-by: Colin Cross 

Has this been applied already?

Rafael


> ---
>  drivers/cpuidle/coupled.c |   12 
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
> index 2c9bf26..c24dda0 100644
> --- a/drivers/cpuidle/coupled.c
> +++ b/drivers/cpuidle/coupled.c
> @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
> notifier_block *nb,
>   int cpu = (unsigned long)hcpu;
>   struct cpuidle_device *dev;
>  
> + switch (action & ~CPU_TASKS_FROZEN) {
> + case CPU_UP_PREPARE:
> + case CPU_DOWN_PREPARE:
> + case CPU_ONLINE:
> + case CPU_DEAD:
> + case CPU_UP_CANCELED:
> + case CPU_DOWN_FAILED:
> + break;
> + default:
> + return NOTIFY_OK;
> + }
> +
>   mutex_lock(_lock);
>  
>   dev = per_cpu(cpuidle_devices, cpu);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-08-07 Thread Rafael J. Wysocki
On Wednesday, July 25, 2012, Colin Cross wrote:
 The cpu hotplug notifier gets called in both atomic and non-atomic
 contexts, it is not always safe to lock a mutex.  Filter out all events
 except the six necessary ones, which are all sleepable, before taking
 the mutex.
 
 Signed-off-by: Colin Cross ccr...@android.com

Has this been applied already?

Rafael


 ---
  drivers/cpuidle/coupled.c |   12 
  1 files changed, 12 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
 index 2c9bf26..c24dda0 100644
 --- a/drivers/cpuidle/coupled.c
 +++ b/drivers/cpuidle/coupled.c
 @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
 notifier_block *nb,
   int cpu = (unsigned long)hcpu;
   struct cpuidle_device *dev;
  
 + switch (action  ~CPU_TASKS_FROZEN) {
 + case CPU_UP_PREPARE:
 + case CPU_DOWN_PREPARE:
 + case CPU_ONLINE:
 + case CPU_DEAD:
 + case CPU_UP_CANCELED:
 + case CPU_DOWN_FAILED:
 + break;
 + default:
 + return NOTIFY_OK;
 + }
 +
   mutex_lock(cpuidle_lock);
  
   dev = per_cpu(cpuidle_devices, cpu);
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-08-07 Thread Colin Cross
On Tue, Aug 7, 2012 at 3:15 PM, Rafael J. Wysocki r...@sisk.pl wrote:
 On Wednesday, July 25, 2012, Colin Cross wrote:
 The cpu hotplug notifier gets called in both atomic and non-atomic
 contexts, it is not always safe to lock a mutex.  Filter out all events
 except the six necessary ones, which are all sleepable, before taking
 the mutex.

 Signed-off-by: Colin Cross ccr...@android.com

 Has this been applied already?

It's not in Linus' tree, and I haven't heard anything from Len.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-08-01 Thread Srivatsa S. Bhat
On 07/31/2012 11:57 PM, Colin Cross wrote:
> On Tue, Jul 31, 2012 at 8:43 AM, Srivatsa S. Bhat
>  wrote:
>> On 07/26/2012 02:50 AM, Colin Cross wrote:
>>> The cpu hotplug notifier gets called in both atomic and non-atomic
>>> contexts, it is not always safe to lock a mutex.  Filter out all events
>>> except the six necessary ones, which are all sleepable, before taking
>>> the mutex.
>>>
>>> Signed-off-by: Colin Cross 
>>> ---
>>>  drivers/cpuidle/coupled.c |   12 
>>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
>>> index 2c9bf26..c24dda0 100644
>>> --- a/drivers/cpuidle/coupled.c
>>> +++ b/drivers/cpuidle/coupled.c
>>> @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
>>> notifier_block *nb,
>>>   int cpu = (unsigned long)hcpu;
>>>   struct cpuidle_device *dev;
>>>
>>> + switch (action & ~CPU_TASKS_FROZEN) {
>>> + case CPU_UP_PREPARE:
>>> + case CPU_DOWN_PREPARE:
>>> + case CPU_ONLINE:
>>> + case CPU_DEAD:
>>> + case CPU_UP_CANCELED:
>>> + case CPU_DOWN_FAILED:
>>> + break;
>>> + default:
>>> + return NOTIFY_OK;
>>> + }
>>> +
>>
>> Instead, wouldn't it be better to have case statements for the
>> 2 cases that imply atomic context and return immediately?
>>
>> Something like:
>> switch (action & ~CPU_TASKS_FROZEN) {
>> case CPU_STARTING:
>> case CPU_DYING:
>> return NOTIFY_OK;
>> }
> 
> No, because then it would need updating whenever a new notification
> event was added.
> 

Hmm.. Fair enough.

Reviewed-by: Srivatsa S. Bhat 

Regards,
Srivatsa S. Bhat

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-08-01 Thread Srivatsa S. Bhat
On 07/31/2012 11:57 PM, Colin Cross wrote:
 On Tue, Jul 31, 2012 at 8:43 AM, Srivatsa S. Bhat
 srivatsa.b...@linux.vnet.ibm.com wrote:
 On 07/26/2012 02:50 AM, Colin Cross wrote:
 The cpu hotplug notifier gets called in both atomic and non-atomic
 contexts, it is not always safe to lock a mutex.  Filter out all events
 except the six necessary ones, which are all sleepable, before taking
 the mutex.

 Signed-off-by: Colin Cross ccr...@android.com
 ---
  drivers/cpuidle/coupled.c |   12 
  1 files changed, 12 insertions(+), 0 deletions(-)

 diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
 index 2c9bf26..c24dda0 100644
 --- a/drivers/cpuidle/coupled.c
 +++ b/drivers/cpuidle/coupled.c
 @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
 notifier_block *nb,
   int cpu = (unsigned long)hcpu;
   struct cpuidle_device *dev;

 + switch (action  ~CPU_TASKS_FROZEN) {
 + case CPU_UP_PREPARE:
 + case CPU_DOWN_PREPARE:
 + case CPU_ONLINE:
 + case CPU_DEAD:
 + case CPU_UP_CANCELED:
 + case CPU_DOWN_FAILED:
 + break;
 + default:
 + return NOTIFY_OK;
 + }
 +

 Instead, wouldn't it be better to have case statements for the
 2 cases that imply atomic context and return immediately?

 Something like:
 switch (action  ~CPU_TASKS_FROZEN) {
 case CPU_STARTING:
 case CPU_DYING:
 return NOTIFY_OK;
 }
 
 No, because then it would need updating whenever a new notification
 event was added.
 

Hmm.. Fair enough.

Reviewed-by: Srivatsa S. Bhat srivatsa.b...@linux.vnet.ibm.com

Regards,
Srivatsa S. Bhat

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-31 Thread Colin Cross
On Tue, Jul 31, 2012 at 8:43 AM, Srivatsa S. Bhat
 wrote:
> On 07/26/2012 02:50 AM, Colin Cross wrote:
>> The cpu hotplug notifier gets called in both atomic and non-atomic
>> contexts, it is not always safe to lock a mutex.  Filter out all events
>> except the six necessary ones, which are all sleepable, before taking
>> the mutex.
>>
>> Signed-off-by: Colin Cross 
>> ---
>>  drivers/cpuidle/coupled.c |   12 
>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
>> index 2c9bf26..c24dda0 100644
>> --- a/drivers/cpuidle/coupled.c
>> +++ b/drivers/cpuidle/coupled.c
>> @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
>> notifier_block *nb,
>>   int cpu = (unsigned long)hcpu;
>>   struct cpuidle_device *dev;
>>
>> + switch (action & ~CPU_TASKS_FROZEN) {
>> + case CPU_UP_PREPARE:
>> + case CPU_DOWN_PREPARE:
>> + case CPU_ONLINE:
>> + case CPU_DEAD:
>> + case CPU_UP_CANCELED:
>> + case CPU_DOWN_FAILED:
>> + break;
>> + default:
>> + return NOTIFY_OK;
>> + }
>> +
>
> Instead, wouldn't it be better to have case statements for the
> 2 cases that imply atomic context and return immediately?
>
> Something like:
> switch (action & ~CPU_TASKS_FROZEN) {
> case CPU_STARTING:
> case CPU_DYING:
> return NOTIFY_OK;
> }

No, because then it would need updating whenever a new notification
event was added.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-31 Thread Srivatsa S. Bhat
On 07/26/2012 02:50 AM, Colin Cross wrote:
> The cpu hotplug notifier gets called in both atomic and non-atomic
> contexts, it is not always safe to lock a mutex.  Filter out all events
> except the six necessary ones, which are all sleepable, before taking
> the mutex.
> 
> Signed-off-by: Colin Cross 
> ---
>  drivers/cpuidle/coupled.c |   12 
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
> index 2c9bf26..c24dda0 100644
> --- a/drivers/cpuidle/coupled.c
> +++ b/drivers/cpuidle/coupled.c
> @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
> notifier_block *nb,
>   int cpu = (unsigned long)hcpu;
>   struct cpuidle_device *dev;
> 
> + switch (action & ~CPU_TASKS_FROZEN) {
> + case CPU_UP_PREPARE:
> + case CPU_DOWN_PREPARE:
> + case CPU_ONLINE:
> + case CPU_DEAD:
> + case CPU_UP_CANCELED:
> + case CPU_DOWN_FAILED:
> + break;
> + default:
> + return NOTIFY_OK;
> + }
> +

Instead, wouldn't it be better to have case statements for the
2 cases that imply atomic context and return immediately?

Something like:
switch (action & ~CPU_TASKS_FROZEN) {
case CPU_STARTING:
case CPU_DYING:
return NOTIFY_OK;
}

Regards,
Srivatsa S. Bhat

>   mutex_lock(_lock);
> 
>   dev = per_cpu(cpuidle_devices, cpu);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-31 Thread Srivatsa S. Bhat
On 07/26/2012 02:50 AM, Colin Cross wrote:
 The cpu hotplug notifier gets called in both atomic and non-atomic
 contexts, it is not always safe to lock a mutex.  Filter out all events
 except the six necessary ones, which are all sleepable, before taking
 the mutex.
 
 Signed-off-by: Colin Cross ccr...@android.com
 ---
  drivers/cpuidle/coupled.c |   12 
  1 files changed, 12 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
 index 2c9bf26..c24dda0 100644
 --- a/drivers/cpuidle/coupled.c
 +++ b/drivers/cpuidle/coupled.c
 @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
 notifier_block *nb,
   int cpu = (unsigned long)hcpu;
   struct cpuidle_device *dev;
 
 + switch (action  ~CPU_TASKS_FROZEN) {
 + case CPU_UP_PREPARE:
 + case CPU_DOWN_PREPARE:
 + case CPU_ONLINE:
 + case CPU_DEAD:
 + case CPU_UP_CANCELED:
 + case CPU_DOWN_FAILED:
 + break;
 + default:
 + return NOTIFY_OK;
 + }
 +

Instead, wouldn't it be better to have case statements for the
2 cases that imply atomic context and return immediately?

Something like:
switch (action  ~CPU_TASKS_FROZEN) {
case CPU_STARTING:
case CPU_DYING:
return NOTIFY_OK;
}

Regards,
Srivatsa S. Bhat

   mutex_lock(cpuidle_lock);
 
   dev = per_cpu(cpuidle_devices, cpu);
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-31 Thread Colin Cross
On Tue, Jul 31, 2012 at 8:43 AM, Srivatsa S. Bhat
srivatsa.b...@linux.vnet.ibm.com wrote:
 On 07/26/2012 02:50 AM, Colin Cross wrote:
 The cpu hotplug notifier gets called in both atomic and non-atomic
 contexts, it is not always safe to lock a mutex.  Filter out all events
 except the six necessary ones, which are all sleepable, before taking
 the mutex.

 Signed-off-by: Colin Cross ccr...@android.com
 ---
  drivers/cpuidle/coupled.c |   12 
  1 files changed, 12 insertions(+), 0 deletions(-)

 diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
 index 2c9bf26..c24dda0 100644
 --- a/drivers/cpuidle/coupled.c
 +++ b/drivers/cpuidle/coupled.c
 @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
 notifier_block *nb,
   int cpu = (unsigned long)hcpu;
   struct cpuidle_device *dev;

 + switch (action  ~CPU_TASKS_FROZEN) {
 + case CPU_UP_PREPARE:
 + case CPU_DOWN_PREPARE:
 + case CPU_ONLINE:
 + case CPU_DEAD:
 + case CPU_UP_CANCELED:
 + case CPU_DOWN_FAILED:
 + break;
 + default:
 + return NOTIFY_OK;
 + }
 +

 Instead, wouldn't it be better to have case statements for the
 2 cases that imply atomic context and return immediately?

 Something like:
 switch (action  ~CPU_TASKS_FROZEN) {
 case CPU_STARTING:
 case CPU_DYING:
 return NOTIFY_OK;
 }

No, because then it would need updating whenever a new notification
event was added.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Shilimkar, Santosh
On Thu, Jul 26, 2012 at 9:25 AM, Shilimkar, Santosh
 wrote:
> On Wed, Jul 25, 2012 at 11:20 PM, Colin Cross  wrote:
>> The cpu hotplug notifier gets called in both atomic and non-atomic
>> contexts, it is not always safe to lock a mutex.  Filter out all events
>> except the six necessary ones, which are all sleepable, before taking
>> the mutex.
>>
>> Signed-off-by: Colin Cross 
>> ---
> Agree.
> Have you observed any lock-up ?
>
Colin explained me about cause of the issue in an off-list discussion.
Thought of updating the thread in case some one wants to reproduce the
issue. You get  a warning during cpu hotplug in suspend if you turn on
sleeping while atomic debugging option in kernel build and the patch
fixes it.

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Rafael J. Wysocki
On Thursday, July 26, 2012, Rafael J. Wysocki wrote:
> On Thursday, July 26, 2012, Colin Cross wrote:
> > On Thu, Jul 26, 2012 at 12:55 PM, Rafael J. Wysocki  wrote:
> > > On Wednesday, July 25, 2012, Colin Cross wrote:
> > >> The cpu hotplug notifier gets called in both atomic and non-atomic
> > >> contexts, it is not always safe to lock a mutex.  Filter out all events
> > >> except the six necessary ones, which are all sleepable, before taking
> > >> the mutex.
> > >
> > > I wonder what mutual exclusion mechanis we rely on when the mutex is not 
> > > taken?
> > 
> > We don't need any mutual exclusion because the notifier returns immediately.
> 
> Don't we need to disable preemption even?

Sorry, scratch that.  It returns NOTIFY_OK if we're not going to take the
mutex.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Rafael J. Wysocki
On Thursday, July 26, 2012, Colin Cross wrote:
> On Thu, Jul 26, 2012 at 12:55 PM, Rafael J. Wysocki  wrote:
> > On Wednesday, July 25, 2012, Colin Cross wrote:
> >> The cpu hotplug notifier gets called in both atomic and non-atomic
> >> contexts, it is not always safe to lock a mutex.  Filter out all events
> >> except the six necessary ones, which are all sleepable, before taking
> >> the mutex.
> >
> > I wonder what mutual exclusion mechanis we rely on when the mutex is not 
> > taken?
> 
> We don't need any mutual exclusion because the notifier returns immediately.

Don't we need to disable preemption even?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Colin Cross
On Thu, Jul 26, 2012 at 12:55 PM, Rafael J. Wysocki  wrote:
> On Wednesday, July 25, 2012, Colin Cross wrote:
>> The cpu hotplug notifier gets called in both atomic and non-atomic
>> contexts, it is not always safe to lock a mutex.  Filter out all events
>> except the six necessary ones, which are all sleepable, before taking
>> the mutex.
>
> I wonder what mutual exclusion mechanis we rely on when the mutex is not 
> taken?

We don't need any mutual exclusion because the notifier returns immediately.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Rafael J. Wysocki
On Wednesday, July 25, 2012, Colin Cross wrote:
> The cpu hotplug notifier gets called in both atomic and non-atomic
> contexts, it is not always safe to lock a mutex.  Filter out all events
> except the six necessary ones, which are all sleepable, before taking
> the mutex.

I wonder what mutual exclusion mechanis we rely on when the mutex is not taken?

Rafael


> Signed-off-by: Colin Cross 
> ---
>  drivers/cpuidle/coupled.c |   12 
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
> index 2c9bf26..c24dda0 100644
> --- a/drivers/cpuidle/coupled.c
> +++ b/drivers/cpuidle/coupled.c
> @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
> notifier_block *nb,
>   int cpu = (unsigned long)hcpu;
>   struct cpuidle_device *dev;
>  
> + switch (action & ~CPU_TASKS_FROZEN) {
> + case CPU_UP_PREPARE:
> + case CPU_DOWN_PREPARE:
> + case CPU_ONLINE:
> + case CPU_DEAD:
> + case CPU_UP_CANCELED:
> + case CPU_DOWN_FAILED:
> + break;
> + default:
> + return NOTIFY_OK;
> + }
> +
>   mutex_lock(_lock);
>  
>   dev = per_cpu(cpuidle_devices, cpu);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Shilimkar, Santosh
On Wed, Jul 25, 2012 at 11:20 PM, Colin Cross  wrote:
> The cpu hotplug notifier gets called in both atomic and non-atomic
> contexts, it is not always safe to lock a mutex.  Filter out all events
> except the six necessary ones, which are all sleepable, before taking
> the mutex.
>
> Signed-off-by: Colin Cross 
> ---
Agree.
Have you observed any lock-up ?

For that patch itself, Acked-by: Santosh Shilimkar 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Shilimkar, Santosh
On Wed, Jul 25, 2012 at 11:20 PM, Colin Cross ccr...@android.com wrote:
 The cpu hotplug notifier gets called in both atomic and non-atomic
 contexts, it is not always safe to lock a mutex.  Filter out all events
 except the six necessary ones, which are all sleepable, before taking
 the mutex.

 Signed-off-by: Colin Cross ccr...@android.com
 ---
Agree.
Have you observed any lock-up ?

For that patch itself, Acked-by: Santosh Shilimkar santosh.shilim...@ti.com
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Rafael J. Wysocki
On Wednesday, July 25, 2012, Colin Cross wrote:
 The cpu hotplug notifier gets called in both atomic and non-atomic
 contexts, it is not always safe to lock a mutex.  Filter out all events
 except the six necessary ones, which are all sleepable, before taking
 the mutex.

I wonder what mutual exclusion mechanis we rely on when the mutex is not taken?

Rafael


 Signed-off-by: Colin Cross ccr...@android.com
 ---
  drivers/cpuidle/coupled.c |   12 
  1 files changed, 12 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/cpuidle/coupled.c b/drivers/cpuidle/coupled.c
 index 2c9bf26..c24dda0 100644
 --- a/drivers/cpuidle/coupled.c
 +++ b/drivers/cpuidle/coupled.c
 @@ -678,6 +678,18 @@ static int cpuidle_coupled_cpu_notify(struct 
 notifier_block *nb,
   int cpu = (unsigned long)hcpu;
   struct cpuidle_device *dev;
  
 + switch (action  ~CPU_TASKS_FROZEN) {
 + case CPU_UP_PREPARE:
 + case CPU_DOWN_PREPARE:
 + case CPU_ONLINE:
 + case CPU_DEAD:
 + case CPU_UP_CANCELED:
 + case CPU_DOWN_FAILED:
 + break;
 + default:
 + return NOTIFY_OK;
 + }
 +
   mutex_lock(cpuidle_lock);
  
   dev = per_cpu(cpuidle_devices, cpu);
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Colin Cross
On Thu, Jul 26, 2012 at 12:55 PM, Rafael J. Wysocki r...@sisk.pl wrote:
 On Wednesday, July 25, 2012, Colin Cross wrote:
 The cpu hotplug notifier gets called in both atomic and non-atomic
 contexts, it is not always safe to lock a mutex.  Filter out all events
 except the six necessary ones, which are all sleepable, before taking
 the mutex.

 I wonder what mutual exclusion mechanis we rely on when the mutex is not 
 taken?

We don't need any mutual exclusion because the notifier returns immediately.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Rafael J. Wysocki
On Thursday, July 26, 2012, Colin Cross wrote:
 On Thu, Jul 26, 2012 at 12:55 PM, Rafael J. Wysocki r...@sisk.pl wrote:
  On Wednesday, July 25, 2012, Colin Cross wrote:
  The cpu hotplug notifier gets called in both atomic and non-atomic
  contexts, it is not always safe to lock a mutex.  Filter out all events
  except the six necessary ones, which are all sleepable, before taking
  the mutex.
 
  I wonder what mutual exclusion mechanis we rely on when the mutex is not 
  taken?
 
 We don't need any mutual exclusion because the notifier returns immediately.

Don't we need to disable preemption even?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Rafael J. Wysocki
On Thursday, July 26, 2012, Rafael J. Wysocki wrote:
 On Thursday, July 26, 2012, Colin Cross wrote:
  On Thu, Jul 26, 2012 at 12:55 PM, Rafael J. Wysocki r...@sisk.pl wrote:
   On Wednesday, July 25, 2012, Colin Cross wrote:
   The cpu hotplug notifier gets called in both atomic and non-atomic
   contexts, it is not always safe to lock a mutex.  Filter out all events
   except the six necessary ones, which are all sleepable, before taking
   the mutex.
  
   I wonder what mutual exclusion mechanis we rely on when the mutex is not 
   taken?
  
  We don't need any mutual exclusion because the notifier returns immediately.
 
 Don't we need to disable preemption even?

Sorry, scratch that.  It returns NOTIFY_OK if we're not going to take the
mutex.

Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier

2012-07-26 Thread Shilimkar, Santosh
On Thu, Jul 26, 2012 at 9:25 AM, Shilimkar, Santosh
santosh.shilim...@ti.com wrote:
 On Wed, Jul 25, 2012 at 11:20 PM, Colin Cross ccr...@android.com wrote:
 The cpu hotplug notifier gets called in both atomic and non-atomic
 contexts, it is not always safe to lock a mutex.  Filter out all events
 except the six necessary ones, which are all sleepable, before taking
 the mutex.

 Signed-off-by: Colin Cross ccr...@android.com
 ---
 Agree.
 Have you observed any lock-up ?

Colin explained me about cause of the issue in an off-list discussion.
Thought of updating the thread in case some one wants to reproduce the
issue. You get  a warning during cpu hotplug in suspend if you turn on
sleeping while atomic debugging option in kernel build and the patch
fixes it.

Regards
Santosh
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/