Re: [PATCH] cpuidle: coupled: fix sleeping while atomic in cpu notifier
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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/