public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Guillaume Autran <gautran@mrv•com>
To: linux-ppc-embedded <linuxppc-embedded@ozlabs•org>
Subject: [PATCH] 8xx: get_mmu_context() for (very) FEW_CONTEXTS and KERNEL_PREEMPT race/starvation issue
Date: Tue, 28 Jun 2005 09:42:57 -0400	[thread overview]
Message-ID: <42C153E1.3060004@mrv.com> (raw)
In-Reply-To: <1119940208.5133.204.camel@gaston>

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

Hi,

I happen to notice a race condition in the mmu_context code for the 8xx 
with very few context (16 MMU contexts) and kernel preemption enable. It 
is hard to reproduce has it shows only when many processes are 
created/destroy and the system is doing a lot of IRQ processing.

In short, one process is trying to steal a context that is in the 
process of being freed (mm->context == NO_CONTEXT) but not completely 
freed (nr_free_contexts == 0).
The steal_context() function does not do anything and the process stays 
in the loop forever.

Anyway, I got a patch that fixes this part. Does not seem to affect 
scheduling latency at all.

Comments are appreciated.

Guillaume.

-- 
=======================================
Guillaume Autran
Senior Software Engineer
MRV Communications, Inc.
Tel: (978) 952-4932 office
E-mail: gautran@mrv•com
======================================= 


[-- Attachment #2: mmu_context.patch --]
[-- Type: text/plain, Size: 3222 bytes --]

diff -Nru --exclude=CVS linux-2.6.old/arch/ppc/mm/mmu_context.c linux-2.6/arch/ppc/mm/mmu_context.c
--- linux-2.6.old/arch/ppc/mm/mmu_context.c	2004-12-13 16:11:56.000000000 -0500
+++ linux-2.6/arch/ppc/mm/mmu_context.c	2005-06-28 09:08:13.000000000 -0400
@@ -31,9 +31,9 @@
 #include <asm/tlbflush.h>
 
 mm_context_t next_mmu_context;
+spinlock_t next_mmu_ctx_lock = SPIN_LOCK_UNLOCKED;
 unsigned long context_map[LAST_CONTEXT / BITS_PER_LONG + 1];
 #ifdef FEW_CONTEXTS
-atomic_t nr_free_contexts;
 struct mm_struct *context_mm[LAST_CONTEXT+1];
 void steal_context(void);
 #endif /* FEW_CONTEXTS */
@@ -52,9 +52,6 @@
 	 */
 	context_map[0] = (1 << FIRST_CONTEXT) - 1;
 	next_mmu_context = FIRST_CONTEXT;
-#ifdef FEW_CONTEXTS
-	atomic_set(&nr_free_contexts, LAST_CONTEXT - FIRST_CONTEXT + 1);
-#endif /* FEW_CONTEXTS */
 }
 
 #ifdef FEW_CONTEXTS
@@ -74,12 +71,21 @@
 steal_context(void)
 {
 	struct mm_struct *mm;
+	mm_context_t ctx = 0;
 
 	/* free up context `next_mmu_context' */
+	spin_lock(&next_mmu_ctx_lock);
+
 	/* if we shouldn't free context 0, don't... */
 	if (next_mmu_context < FIRST_CONTEXT)
 		next_mmu_context = FIRST_CONTEXT;
-	mm = context_mm[next_mmu_context];
+
+	ctx = next_mmu_context;
+	next_mmu_context = (ctx + 1) & LAST_CONTEXT;
+
+	spin_unlock(&next_mmu_ctx_lock);
+
+	mm = context_mm[ctx];
 	flush_tlb_mm(mm);
 	destroy_context(mm);
 }
diff -Nru --exclude=CVS linux-2.6.old/include/asm-ppc/mmu_context.h linux-2.6/include/asm-ppc/mmu_context.h
--- linux-2.6.old/include/asm-ppc/mmu_context.h	2004-12-13 16:11:21.000000000 -0500
+++ linux-2.6/include/asm-ppc/mmu_context.h	2005-06-28 09:08:13.000000000 -0400
@@ -100,6 +100,7 @@
  * number to be free, but it usually will be.
  */
 extern mm_context_t next_mmu_context;
+extern spinlock_t next_mmu_ctx_lock;
 
 /*
  * If we don't have sufficient contexts to give one to every task
@@ -108,7 +109,6 @@
  */
 #if LAST_CONTEXT < 30000
 #define FEW_CONTEXTS	1
-extern atomic_t nr_free_contexts;
 extern struct mm_struct *context_mm[LAST_CONTEXT+1];
 extern void steal_context(void);
 #endif
@@ -119,24 +119,36 @@
 static inline void get_mmu_context(struct mm_struct *mm)
 {
 	mm_context_t ctx;
+	int flag;
 
 	if (mm->context != NO_CONTEXT)
 		return;
-#ifdef FEW_CONTEXTS
-	while (atomic_dec_if_positive(&nr_free_contexts) < 0)
-		steal_context();
-#endif
+
 	ctx = next_mmu_context;
+	flag = 0;
+
 	while (test_and_set_bit(ctx, context_map)) {
 		ctx = find_next_zero_bit(context_map, LAST_CONTEXT+1, ctx);
-		if (ctx > LAST_CONTEXT)
+		if (ctx > LAST_CONTEXT) {
 			ctx = 0;
+#ifdef FEW_CONTEXTS
+			if( flag == 0 ) {
+				flag = 1;
+			} else {	
+				ctx = next_mmu_context;
+				steal_context();
+			}
+#endif
+		}
 	}
+	spin_lock(&next_mmu_ctx_lock);
 	next_mmu_context = (ctx + 1) & LAST_CONTEXT;
 	mm->context = ctx;
 #ifdef FEW_CONTEXTS
 	context_mm[ctx] = mm;
 #endif
+	spin_unlock(&next_mmu_ctx_lock);
+
 }
 
 /*
@@ -150,11 +162,9 @@
 static inline void destroy_context(struct mm_struct *mm)
 {
 	if (mm->context != NO_CONTEXT) {
-		clear_bit(mm->context, context_map);
+		mm_context_t ctx = mm->context;
 		mm->context = NO_CONTEXT;
-#ifdef FEW_CONTEXTS
-		atomic_inc(&nr_free_contexts);
-#endif
+		clear_bit(ctx, context_map);
 	}
 }
 

  reply	other threads:[~2005-06-28 13:41 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-06-25 14:53 [PATCH] 8xx: map_page() skip pinned region and tlbie debugging aid Marcelo Tosatti
2005-06-25 22:24 ` Dan Malek
2005-06-26 14:30   ` Marcelo Tosatti
2005-06-27 13:39     ` Marcelo Tosatti
2005-06-27 20:46       ` Dan Malek
2005-06-28  6:30         ` Benjamin Herrenschmidt
2005-06-28 13:42           ` Guillaume Autran [this message]
2005-06-29  4:15             ` [PATCH] 8xx: get_mmu_context() for (very) FEW_CONTEXTS and KERNEL_PREEMPT race/starvation issue Benjamin Herrenschmidt
2005-06-29 15:32               ` Guillaume Autran
2005-06-29 15:54                 ` Marcelo Tosatti
2005-06-29 21:25                   ` Guillaume Autran
2005-06-29 17:00                     ` Marcelo Tosatti
2005-06-29 23:26                   ` Benjamin Herrenschmidt
2005-06-29 19:38                     ` Marcelo Tosatti
2005-06-30 13:54                       ` Guillaume Autran
2005-07-05 13:12                         ` Guillaume Autran
2005-06-30  0:34                     ` Eugene Surovegin
2005-06-29 23:24                 ` Benjamin Herrenschmidt
2005-06-28 13:53           ` [PATCH] 8xx: map_page() skip pinned region and tlbie debugging aid Dan Malek
2005-06-28 23:47             ` Benjamin Herrenschmidt
2005-06-29 17:19             ` Marcelo Tosatti
2005-06-29 23:31               ` Benjamin Herrenschmidt
2005-06-30 18:05                 ` Dan Malek
2005-06-30 23:29                   ` Benjamin Herrenschmidt
2005-07-01  7:01                     ` Pantelis Antoniou
2005-06-30 17:49               ` Dan Malek
2005-06-27 14:28   ` [PATCH] 8xx: tlbie debugging aid (try #2) Marcelo Tosatti
2005-06-27 20:18     ` Dan Malek
2005-06-27 14:56       ` Marcelo Tosatti
2005-06-27 20:53         ` Dan Malek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=42C153E1.3060004@mrv.com \
    --to=gautran@mrv$(echo .)com \
    --cc=linuxppc-embedded@ozlabs$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox