Re: [PATCH 3/6] i386 virtualization - Make ldt a desc struct

From: Chuck Ebbert
Date: Tue Aug 16 2005 - 12:06:47 EST


On Mon, 15 Aug 2005 at 15:59:39 -0700, zach@xxxxxxxxxx wrote:

> --- linux-2.6.13.orig/include/asm-i386/mmu_context.h 2005-08-15 11:16:59.000000000 -0700
> +++ linux-2.6.13/include/asm-i386/mmu_context.h 2005-08-15 11:19:49.000000000 -0700
> @@ -19,7 +19,7 @@
> memset(&mm->context, 0, sizeof(mm->context));
> init_MUTEX(&mm->context.sem);
> old_mm = current->mm;
> - if (old_mm && unlikely(old_mm->context.size > 0)) {
> + if (old_mm && unlikely(old_mm->context.ldt)) { <==================
> retval = copy_ldt(&mm->context, &old_mm->context);
> }
> if (retval == 0)
> @@ -32,7 +32,7 @@
> */
> static inline void destroy_context(struct mm_struct *mm)
> {
> - if (unlikely(mm->context.size))
> + if (unlikely(mm->context.ldt)) <==================
> destroy_ldt(mm);
> del_lazy_mm(mm);
> }

Here you changed the code so it no longer tests the size field, however:

> --- linux-2.6.13.orig/arch/i386/kernel/ldt.c 2005-08-15 11:16:59.000000000 -0700
> +++ linux-2.6.13/arch/i386/kernel/ldt.c 2005-08-15 11:19:49.000000000 -0700
<--SNIP-->
> @@ -97,14 +96,16 @@
>
> void destroy_ldt(struct mm_struct *mm)
> {
> + int pages = mm->context.ldt_pages;
> +
> if (mm == current->active_mm)
> clear_LDT();
> - ClearPagesLDT(mm->context.ldt, (mm->context.size * LDT_ENTRY_SIZE) / PAGE_SIZE);
> - if (mm->context.size*LDT_ENTRY_SIZE > PAGE_SIZE)
> + ClearPagesLDT(mm->context.ldt, pages);
> + if (pages > 1)
> vfree(mm->context.ldt);
> else
> kfree(mm->context.ldt);
> - mm->context.size = 0;
> + mm->context.ldt_pages = 0; <====================
> }
>
> static int read_ldt(void __user * ptr, unsigned long bytecount)

destroy_ldt does not zero "ldt", just the size. Potential bug?

Also:

> --- linux-2.6.13.orig/arch/i386/kernel/ldt.c 2005-08-15 11:16:59.000000000 -0700
> +++ linux-2.6.13/arch/i386/kernel/ldt.c 2005-08-15 11:19:49.000000000 -0700
> @@ -28,28 +28,27 @@
> }
> #endif
>
> -static inline int alloc_ldt(mm_context_t *pc, const int oldsize, int mincount, const int reload)
> +static inline int alloc_ldt(mm_context_t *pc, const int old_pages, int new_pages, const int reload)
> {
> - void *oldldt;
> - void *newldt;
> + struct desc_struct *oldldt;
> + struct desc_struct *newldt;

Can't this be declared on one line?


...and BTW could you add:

QUILT_DIFF_OPTS=-p

to your shell env? It makes the patches much easier to review.

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