Skip to contentSkip to conversation

preempt-safe load_LDT

PATCH In review Robert Love 3 participants 3 messages Jul 20, 2002

lore
Share & tools
lore

Robert Love

author
#0 lore raw

PATCH 1/1 preempt-safe load_LDT

Linus,

Attached patch makes load_LDT preempt-safe per our off-list discussion.

Header dependencies prevent use of the get_cpu()/put_cpu() methods so we explicitly call disable/enable.

This is Ingo Molnar's version of the patch: I like how he broke up load_LDT into two versions better.

Patch is against 2.5.27, please apply.

Robert Love

include/asm-i386/desc.h +9 -2
diff -urN linux-2.5.27/include/asm-i386/desc.h linux/include/asm-i386/desc.h
@@ -90,9 +90,8 @@
 /*
  * load one particular LDT into the current CPU
  */
-static inline void load_LDT (mm_context_t *pc)
+static inline void load_LDT_no_preempt(mm_context_t *pc, int cpu)
 {
-	int cpu = smp_processor_id();
 	void *segments = pc->ldt;
 	int count = pc->size;
 

@@ -104,6 +103,14 @@
 	set_ldt_desc(cpu, segments, count);
 	__load_LDT(cpu);
 }
+static inline void load_LDT(mm_context_t *pc)
+{
+	int cpu = smp_processor_id();
+
+	preempt_disable();
+	load_LDT_no_preempt(pc, cpu);
+	preempt_enable();
+}
 
 #endif /* !__ASSEMBLY__ */
include/asm-i386/mmu_context.h +2 -2
diff -urN linux-2.5.27/include/asm-i386/mmu_context.h linux/include/asm-i386/mmu_context.h
@@ -44,7 +44,7 @@
 		 * has a non-default LDT.
 		 */
 		if (next->context.size+prev->context.size)
-			load_LDT(&next->context);
+			load_LDT_no_preempt(&next->context, cpu);
 	}
 #ifdef CONFIG_SMP
 	else {

@@ -56,7 +56,7 @@
 			 * tlb flush IPI delivery. We must reload %cr3.
 			 */
 			load_cr3(next->pgd);
-			load_LDT(&next->context);
+			load_LDT_no_preempt(&next->context, cpu);
 		}
 	}
 #endif
#1 lore raw
Quoted 2 lines · Robert Love

Attached patch makes load_LDT preempt-safe per our off-list discussion.

It's buggy. It calls smp_processor_id() outside the preemption window, see "load_LDT()".

I'd suggest making it use get/put_cpu() and to avoid the header file dependencies moving it away from desc.h and into ldt.c instead.

Linus

#2 lore raw
Quoted 2 lines · Linus Torvalds

It's buggy. It calls smp_processor_id() outside the preemption window, see "load_LDT()".

oops. Correct patch that uses get_cpu()/put_cpu() is attached, against 2.5.27. I was wrong when sending the initial fix: there are no header file dependency issues in 2.5.27 anymore. It compiles & boots.

Ingo

include/asm-i386/mmu_context.h renamed from include/asm-i386/mmu_context.h.orig +2 -2
@@ -44,7 +44,7 @@
 		 * has a non-default LDT.
 		 */
 		if (next->context.size+prev->context.size)
-			load_LDT(&next->context);
+			load_LDT_no_preempt(&next->context, cpu);
 	}
 #ifdef CONFIG_SMP
 	else {

@@ -56,7 +56,7 @@
 			 * tlb flush IPI delivery. We must reload %cr3.
 			 */
 			load_cr3(next->pgd);
-			load_LDT(&next->context);
+			load_LDT_no_preempt(&next->context, cpu);
 		}
 	}
 #endif
include/asm-i386/desc.h renamed from include/asm-i386/desc.h.orig +8 -2
@@ -90,9 +90,8 @@
 /*
  * load one particular LDT into the current CPU
  */
-static inline void load_LDT (mm_context_t *pc)
+static inline void load_LDT_no_preempt (mm_context_t *pc, int cpu)
 {
-	int cpu = smp_processor_id();
 	void *segments = pc->ldt;
 	int count = pc->size;
 

@@ -103,6 +102,13 @@
 		
 	set_ldt_desc(cpu, segments, count);
 	__load_LDT(cpu);
+}
+static inline void load_LDT (mm_context_t *pc)
+{
+	int cpu = get_cpu();
+
+	load_LDT_no_preempt(pc, cpu);
+	put_cpu();
 }
 
 #endif /* !__ASSEMBLY__ */