* [PATCH] ppc32: fix destroy_context() race condition
@ 2005-07-12 14:23 Guillaume Autran
2005-07-12 16:39 ` Eugene Surovegin
2005-07-13 0:09 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 5+ messages in thread
From: Guillaume Autran @ 2005-07-12 14:23 UTC (permalink / raw)
To: akpm; +Cc: linuxppc-embedded
Fix for a race condition when a task gets preempted by another task
while executing the destroy_context(...) in a FEW_CONTEXTS environment.
mm->context == NO_CONTEXT but the context_map may indicate all contexts
are in use.
The solution to this problem is to disable kernel preemption while
destroying a MMU context.
Signed-off-by: Guillaume Autran <gautran@mrv.com>
---
diff -Nru a/include/asm-ppc/mmu_context.h b/include/asm-ppc/mmu_context.h
--- a/include/asm-ppc/mmu_context.h 2005-06-17 15:48:29.000000000 -0400
+++ b/include/asm-ppc/mmu_context.h 2005-07-05 08:58:46.000000000 -0400
@@ -149,6 +149,7 @@
*/
static inline void destroy_context(struct mm_struct *mm)
{
+ preempt_disable();
if (mm->context != NO_CONTEXT) {
clear_bit(mm->context, context_map);
mm->context = NO_CONTEXT;
@@ -156,6 +157,7 @@
atomic_inc(&nr_free_contexts);
#endif
}
+ preempt_enable();
}
static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ppc32: fix destroy_context() race condition
2005-07-12 14:23 [PATCH] ppc32: fix destroy_context() race condition Guillaume Autran
@ 2005-07-12 16:39 ` Eugene Surovegin
2005-07-12 18:29 ` Guillaume Autran
2005-07-13 0:09 ` Benjamin Herrenschmidt
1 sibling, 1 reply; 5+ messages in thread
From: Eugene Surovegin @ 2005-07-12 16:39 UTC (permalink / raw)
To: Guillaume Autran; +Cc: akpm, linuxppc-embedded
On Tue, Jul 12, 2005 at 10:23:51AM -0400, Guillaume Autran wrote:
> Fix for a race condition when a task gets preempted by another task
> while executing the destroy_context(...) in a FEW_CONTEXTS environment.
> mm->context == NO_CONTEXT but the context_map may indicate all contexts
> are in use.
> The solution to this problem is to disable kernel preemption while
> destroying a MMU context.
>
> Signed-off-by: Guillaume Autran <gautran@mrv.com>
>
> ---
>
> diff -Nru a/include/asm-ppc/mmu_context.h b/include/asm-ppc/mmu_context.h
> --- a/include/asm-ppc/mmu_context.h 2005-06-17 15:48:29.000000000 -0400
> +++ b/include/asm-ppc/mmu_context.h 2005-07-05 08:58:46.000000000 -0400
> @@ -149,6 +149,7 @@
> */
> static inline void destroy_context(struct mm_struct *mm)
> {
> + preempt_disable();
> if (mm->context != NO_CONTEXT) {
> clear_bit(mm->context, context_map);
> mm->context = NO_CONTEXT;
> @@ -156,6 +157,7 @@
> atomic_inc(&nr_free_contexts);
> #endif
> }
> + preempt_enable();
> }
>
> static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>
Could you explain, why this patch is needed?
AFAIK, destroy_context() is only called from switch_mm(), which in
turn is called from schedule() with preemption already disabled. If
not, IMHO we have bigger problems, and "fixing" destroy_context() will
only hide such problems.
--
Eugene
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ppc32: fix destroy_context() race condition
2005-07-12 16:39 ` Eugene Surovegin
@ 2005-07-12 18:29 ` Guillaume Autran
2005-07-12 18:45 ` Eugene Surovegin
0 siblings, 1 reply; 5+ messages in thread
From: Guillaume Autran @ 2005-07-12 18:29 UTC (permalink / raw)
To: Eugene Surovegin; +Cc: akpm, linuxppc-embedded
[-- Attachment #1: Type: text/plain, Size: 1845 bytes --]
Eugene, destroy_context(..) can be called from other place outside of
the scheduler and switch_mm(...). See mmdrop(...) for example.
In many of those cases, the task will run with preemption enable.
Guillaume.
Eugene Surovegin wrote:
>On Tue, Jul 12, 2005 at 10:23:51AM -0400, Guillaume Autran wrote:
>
>
>>Fix for a race condition when a task gets preempted by another task
>>while executing the destroy_context(...) in a FEW_CONTEXTS environment.
>>mm->context == NO_CONTEXT but the context_map may indicate all contexts
>>are in use.
>>The solution to this problem is to disable kernel preemption while
>>destroying a MMU context.
>>
>>Signed-off-by: Guillaume Autran <gautran@mrv.com>
>>
>>---
>>
>>diff -Nru a/include/asm-ppc/mmu_context.h b/include/asm-ppc/mmu_context.h
>>--- a/include/asm-ppc/mmu_context.h 2005-06-17 15:48:29.000000000 -0400
>>+++ b/include/asm-ppc/mmu_context.h 2005-07-05 08:58:46.000000000 -0400
>>@@ -149,6 +149,7 @@
>>*/
>>static inline void destroy_context(struct mm_struct *mm)
>>{
>>+ preempt_disable();
>> if (mm->context != NO_CONTEXT) {
>> clear_bit(mm->context, context_map);
>> mm->context = NO_CONTEXT;
>>@@ -156,6 +157,7 @@
>> atomic_inc(&nr_free_contexts);
>>#endif
>> }
>>+ preempt_enable();
>>}
>>
>>static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>>
>>
>>
>
>Could you explain, why this patch is needed?
>
>AFAIK, destroy_context() is only called from switch_mm(), which in
>turn is called from schedule() with preemption already disabled. If
>not, IMHO we have bigger problems, and "fixing" destroy_context() will
>only hide such problems.
>
>
>
--
=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: gautran@mrv.com
=======================================
[-- Attachment #2: Type: text/html, Size: 2432 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] ppc32: fix destroy_context() race condition
2005-07-12 18:29 ` Guillaume Autran
@ 2005-07-12 18:45 ` Eugene Surovegin
0 siblings, 0 replies; 5+ messages in thread
From: Eugene Surovegin @ 2005-07-12 18:45 UTC (permalink / raw)
To: Guillaume Autran; +Cc: akpm, linuxppc-embedded
On Tue, Jul 12, 2005 at 02:29:04PM -0400, Guillaume Autran wrote:
> Eugene, destroy_context(..) can be called from other place outside of
> the scheduler and switch_mm(...). See mmdrop(...) for example.
> In many of those cases, the task will run with preemption enable.
Ahh, OK, you're right. Maybe it's worth mentioning in the patch
description :).
--
Eugene
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ppc32: fix destroy_context() race condition
2005-07-12 14:23 [PATCH] ppc32: fix destroy_context() race condition Guillaume Autran
2005-07-12 16:39 ` Eugene Surovegin
@ 2005-07-13 0:09 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2005-07-13 0:09 UTC (permalink / raw)
To: Guillaume Autran; +Cc: akpm, linuxppc-embedded
On Tue, 2005-07-12 at 10:23 -0400, Guillaume Autran wrote:
> Fix for a race condition when a task gets preempted by another task
> while executing the destroy_context(...) in a FEW_CONTEXTS environment.
> mm->context == NO_CONTEXT but the context_map may indicate all contexts
> are in use.
> The solution to this problem is to disable kernel preemption while
> destroying a MMU context.
>
> Signed-off-by: Guillaume Autran <gautran@mrv.com>
Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> ---
>
> diff -Nru a/include/asm-ppc/mmu_context.h b/include/asm-ppc/mmu_context.h
> --- a/include/asm-ppc/mmu_context.h 2005-06-17 15:48:29.000000000 -0400
> +++ b/include/asm-ppc/mmu_context.h 2005-07-05 08:58:46.000000000 -0400
> @@ -149,6 +149,7 @@
> */
> static inline void destroy_context(struct mm_struct *mm)
> {
> + preempt_disable();
> if (mm->context != NO_CONTEXT) {
> clear_bit(mm->context, context_map);
> mm->context = NO_CONTEXT;
> @@ -156,6 +157,7 @@
> atomic_inc(&nr_free_contexts);
> #endif
> }
> + preempt_enable();
> }
>
> static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
>
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2005-07-13 0:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-07-12 14:23 [PATCH] ppc32: fix destroy_context() race condition Guillaume Autran
2005-07-12 16:39 ` Eugene Surovegin
2005-07-12 18:29 ` Guillaume Autran
2005-07-12 18:45 ` Eugene Surovegin
2005-07-13 0:09 ` Benjamin Herrenschmidt
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).