Don't rely on kmalloc(PAGE_SIZE) returning PAGE_SIZE aligned memory
(Xen requires GDT *and* LDT to be page-aligned). Using the page
allocator interface also removes the (albeit small) slab allocator
overhead. The same change being done for 64-bits for consistency.
Further, the Xen hypercall interface expects the LDT address to be
virtual, not machine.
Signed-off-by: Jan Beulich <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
arch/x86/kernel/ldt_32.c | 7 +++----
arch/x86/kernel/ldt_64.c | 7 +++----
arch/x86/xen/enlighten.c | 9 +--------
3 files changed, 7 insertions(+), 16 deletions(-)
--- linux-2.6.24-rc7/arch/x86/kernel/ldt_32.c 2008-01-10 16:53:54.000000000 +0100
+++ 2.6.24-rc7-x86-xen-ldt/arch/x86/kernel/ldt_32.c 2008-01-09 13:59:50.000000000 +0100
@@ -9,7 +9,6 @@
#include <linux/mm.h>
#include <linux/smp.h>
#include <linux/vmalloc.h>
-#include <linux/slab.h>
#include <asm/uaccess.h>
#include <asm/system.h>
@@ -38,7 +37,7 @@ static int alloc_ldt(mm_context_t *pc, i
if (mincount*LDT_ENTRY_SIZE > PAGE_SIZE)
newldt = vmalloc(mincount*LDT_ENTRY_SIZE);
else
- newldt = kmalloc(mincount*LDT_ENTRY_SIZE, GFP_KERNEL);
+ newldt = (void *)__get_free_page(GFP_KERNEL);
if (!newldt)
return -ENOMEM;
@@ -69,7 +68,7 @@ static int alloc_ldt(mm_context_t *pc, i
if (oldsize*LDT_ENTRY_SIZE > PAGE_SIZE)
vfree(oldldt);
else
- kfree(oldldt);
+ put_page(virt_to_page(oldldt));
}
return 0;
}
@@ -114,7 +113,7 @@ void destroy_context(struct mm_struct *m
if (mm->context.size*LDT_ENTRY_SIZE > PAGE_SIZE)
vfree(mm->context.ldt);
else
- kfree(mm->context.ldt);
+ put_page(virt_to_page(mm->context.ldt));
mm->context.size = 0;
}
}
--- linux-2.6.24-rc7/arch/x86/kernel/ldt_64.c 2008-01-10 16:53:54.000000000 +0100
+++ 2.6.24-rc7-x86-xen-ldt/arch/x86/kernel/ldt_64.c 2008-01-09 13:59:50.000000000 +0100
@@ -12,7 +12,6 @@
#include <linux/mm.h>
#include <linux/smp.h>
#include <linux/vmalloc.h>
-#include <linux/slab.h>
#include <asm/uaccess.h>
#include <asm/system.h>
@@ -41,7 +40,7 @@ static int alloc_ldt(mm_context_t *pc, u
if (mincount*LDT_ENTRY_SIZE > PAGE_SIZE)
newldt = vmalloc(mincount*LDT_ENTRY_SIZE);
else
- newldt = kmalloc(mincount*LDT_ENTRY_SIZE, GFP_KERNEL);
+ newldt = (void *)__get_free_page(GFP_KERNEL);
if (!newldt)
return -ENOMEM;
@@ -73,7 +72,7 @@ static int alloc_ldt(mm_context_t *pc, u
if (oldsize*LDT_ENTRY_SIZE > PAGE_SIZE)
vfree(oldldt);
else
- kfree(oldldt);
+ put_page(virt_to_page(oldldt));
}
return 0;
}
@@ -117,7 +116,7 @@ void destroy_context(struct mm_struct *m
if ((unsigned)mm->context.size*LDT_ENTRY_SIZE > PAGE_SIZE)
vfree(mm->context.ldt);
else
- kfree(mm->context.ldt);
+ put_page(virt_to_page(mm->context.ldt));
mm->context.size = 0;
}
}
--- linux-2.6.24-rc7/arch/x86/xen/enlighten.c 2008-01-10 16:53:55.000000000 +0100
+++ 2.6.24-rc7-x86-xen-ldt/arch/x86/xen/enlighten.c 2008-01-09 14:00:51.000000000 +0100
@@ -275,19 +275,12 @@ static unsigned long xen_store_tr(void)
static void xen_set_ldt(const void *addr, unsigned entries)
{
- unsigned long linear_addr = (unsigned long)addr;
struct mmuext_op *op;
struct multicall_space mcs = xen_mc_entry(sizeof(*op));
op = mcs.args;
op->cmd = MMUEXT_SET_LDT;
- if (linear_addr) {
- /* ldt my be vmalloced, use arbitrary_virt_to_machine */
- xmaddr_t maddr;
- maddr = arbitrary_virt_to_machine((unsigned long)addr);
- linear_addr = (unsigned long)maddr.maddr;
- }
- op->arg1.linear_addr = linear_addr;
+ op->arg1.linear_addr = (unsigned long)addr;
op->arg2.nr_ents = entries;
MULTI_mmuext_op(mcs.mc, op, 1, NULL, DOMID_SELF);
On 01/11/2008 10:22 AM, Jan Beulich wrote:
> Don't rely on kmalloc(PAGE_SIZE) returning PAGE_SIZE aligned memory
> (Xen requires GDT *and* LDT to be page-aligned). Using the page
> allocator interface also removes the (albeit small) slab allocator
> overhead. The same change being done for 64-bits for consistency.
>
> Further, the Xen hypercall interface expects the LDT address to be
> virtual, not machine.
>
> Signed-off-by: Jan Beulich <[email protected]>
> Cc: Ingo Molnar <[email protected]>
>
> ---
> arch/x86/kernel/ldt_32.c | 7 +++----
> arch/x86/kernel/ldt_64.c | 7 +++----
> arch/x86/xen/enlighten.c | 9 +--------
> 3 files changed, 7 insertions(+), 16 deletions(-)
>
> --- linux-2.6.24-rc7/arch/x86/kernel/ldt_32.c 2008-01-10 16:53:54.000000000 +0100
> +++ 2.6.24-rc7-x86-xen-ldt/arch/x86/kernel/ldt_32.c 2008-01-09 13:59:50.000000000 +0100
[...]
> @@ -73,7 +72,7 @@ static int alloc_ldt(mm_context_t *pc, u
> if (oldsize*LDT_ENTRY_SIZE > PAGE_SIZE)
> vfree(oldldt);
> else
> - kfree(oldldt);
> + put_page(virt_to_page(oldldt));
> }
> return 0;
> }
> @@ -117,7 +116,7 @@ void destroy_context(struct mm_struct *m
> if ((unsigned)mm->context.size*LDT_ENTRY_SIZE > PAGE_SIZE)
> vfree(mm->context.ldt);
> else
> - kfree(mm->context.ldt);
> + put_page(virt_to_page(mm->context.ldt));
why not free_page() with all those checks if it is correct virt address which it
brings?
regards,
--
Jiri Slaby
Faculty of Informatics, Masaryk University
Suse Labs
>> --- linux-2.6.24-rc7/arch/x86/kernel/ldt_32.c 2008-01-10 16:53:54.000000000 +0100
>> +++ 2.6.24-rc7-x86-xen-ldt/arch/x86/kernel/ldt_32.c 2008-01-09 13:59:50.000000000 +0100
>[...]
>> @@ -73,7 +72,7 @@ static int alloc_ldt(mm_context_t *pc, u
>> if (oldsize*LDT_ENTRY_SIZE > PAGE_SIZE)
>> vfree(oldldt);
>> else
>> - kfree(oldldt);
>> + put_page(virt_to_page(oldldt));
>> }
>> return 0;
>> }
>> @@ -117,7 +116,7 @@ void destroy_context(struct mm_struct *m
>> if ((unsigned)mm->context.size*LDT_ENTRY_SIZE > PAGE_SIZE)
>> vfree(mm->context.ldt);
>> else
>> - kfree(mm->context.ldt);
>> + put_page(virt_to_page(mm->context.ldt));
>
>why not free_page() with all those checks if it is correct virt address which it
>brings?
I followed the combination I found elsewhere - __get_free_page/put_page
or alloc_page/free_page. I admit that I'm unclear about when/why to use
one vs. the other pair.
Jan
Jan Beulich wrote:
> Don't rely on kmalloc(PAGE_SIZE) returning PAGE_SIZE aligned memory
> (Xen requires GDT *and* LDT to be page-aligned).
Can kmalloc return non-page-aligned PAGE_SIZE allocations?
> Using the page
> allocator interface also removes the (albeit small) slab allocator
> overhead.
Runtime or space overhead? Given that they're once-off allocations, the
time part isn't a big factor. And apparently LDT is completely unused.
Seems like a reasonable change either way.
> The same change being done for 64-bits for consistency.
>
> Further, the Xen hypercall interface expects the LDT address to be
> virtual, not machine.
>
Shows how much the LDT gets used then... Hm, but I didn't make that
code up; I wonder where it came from? But certainly, passing the mfn of
the first page of a vmalloc area doesn't make any sense.
> Signed-off-by: Jan Beulich <[email protected]>
> Cc: Ingo Molnar <[email protected]>
>
Acked-by: Jeremy Fitzhardinge <[email protected]>
> ---
> arch/x86/kernel/ldt_32.c | 7 +++----
> arch/x86/kernel/ldt_64.c | 7 +++----
> arch/x86/xen/enlighten.c | 9 +--------
> 3 files changed, 7 insertions(+), 16 deletions(-)
>
> --- linux-2.6.24-rc7/arch/x86/kernel/ldt_32.c 2008-01-10 16:53:54.000000000 +0100
> +++ 2.6.24-rc7-x86-xen-ldt/arch/x86/kernel/ldt_32.c 2008-01-09 13:59:50.000000000 +0100
> @@ -9,7 +9,6 @@
> #include <linux/mm.h>
> #include <linux/smp.h>
> #include <linux/vmalloc.h>
> -#include <linux/slab.h>
>
> #include <asm/uaccess.h>
> #include <asm/system.h>
> @@ -38,7 +37,7 @@ static int alloc_ldt(mm_context_t *pc, i
> if (mincount*LDT_ENTRY_SIZE > PAGE_SIZE)
> newldt = vmalloc(mincount*LDT_ENTRY_SIZE);
> else
> - newldt = kmalloc(mincount*LDT_ENTRY_SIZE, GFP_KERNEL);
> + newldt = (void *)__get_free_page(GFP_KERNEL);
>
> if (!newldt)
> return -ENOMEM;
> @@ -69,7 +68,7 @@ static int alloc_ldt(mm_context_t *pc, i
> if (oldsize*LDT_ENTRY_SIZE > PAGE_SIZE)
> vfree(oldldt);
> else
> - kfree(oldldt);
> + put_page(virt_to_page(oldldt));
> }
> return 0;
> }
> @@ -114,7 +113,7 @@ void destroy_context(struct mm_struct *m
> if (mm->context.size*LDT_ENTRY_SIZE > PAGE_SIZE)
> vfree(mm->context.ldt);
> else
> - kfree(mm->context.ldt);
> + put_page(virt_to_page(mm->context.ldt));
> mm->context.size = 0;
> }
> }
> --- linux-2.6.24-rc7/arch/x86/kernel/ldt_64.c 2008-01-10 16:53:54.000000000 +0100
> +++ 2.6.24-rc7-x86-xen-ldt/arch/x86/kernel/ldt_64.c 2008-01-09 13:59:50.000000000 +0100
> @@ -12,7 +12,6 @@
> #include <linux/mm.h>
> #include <linux/smp.h>
> #include <linux/vmalloc.h>
> -#include <linux/slab.h>
>
> #include <asm/uaccess.h>
> #include <asm/system.h>
> @@ -41,7 +40,7 @@ static int alloc_ldt(mm_context_t *pc, u
> if (mincount*LDT_ENTRY_SIZE > PAGE_SIZE)
> newldt = vmalloc(mincount*LDT_ENTRY_SIZE);
> else
> - newldt = kmalloc(mincount*LDT_ENTRY_SIZE, GFP_KERNEL);
> + newldt = (void *)__get_free_page(GFP_KERNEL);
>
> if (!newldt)
> return -ENOMEM;
> @@ -73,7 +72,7 @@ static int alloc_ldt(mm_context_t *pc, u
> if (oldsize*LDT_ENTRY_SIZE > PAGE_SIZE)
> vfree(oldldt);
> else
> - kfree(oldldt);
> + put_page(virt_to_page(oldldt));
> }
> return 0;
> }
> @@ -117,7 +116,7 @@ void destroy_context(struct mm_struct *m
> if ((unsigned)mm->context.size*LDT_ENTRY_SIZE > PAGE_SIZE)
> vfree(mm->context.ldt);
> else
> - kfree(mm->context.ldt);
> + put_page(virt_to_page(mm->context.ldt));
> mm->context.size = 0;
> }
> }
> --- linux-2.6.24-rc7/arch/x86/xen/enlighten.c 2008-01-10 16:53:55.000000000 +0100
> +++ 2.6.24-rc7-x86-xen-ldt/arch/x86/xen/enlighten.c 2008-01-09 14:00:51.000000000 +0100
> @@ -275,19 +275,12 @@ static unsigned long xen_store_tr(void)
>
> static void xen_set_ldt(const void *addr, unsigned entries)
> {
> - unsigned long linear_addr = (unsigned long)addr;
> struct mmuext_op *op;
> struct multicall_space mcs = xen_mc_entry(sizeof(*op));
>
> op = mcs.args;
> op->cmd = MMUEXT_SET_LDT;
> - if (linear_addr) {
> - /* ldt my be vmalloced, use arbitrary_virt_to_machine */
> - xmaddr_t maddr;
> - maddr = arbitrary_virt_to_machine((unsigned long)addr);
> - linear_addr = (unsigned long)maddr.maddr;
> - }
> - op->arg1.linear_addr = linear_addr;
> + op->arg1.linear_addr = (unsigned long)addr;
> op->arg2.nr_ents = entries;
>
> MULTI_mmuext_op(mcs.mc, op, 1, NULL, DOMID_SELF);
>
>
>
>
>>> Jeremy Fitzhardinge <[email protected]> 11.01.08 18:28 >>>
>Jan Beulich wrote:
>> Don't rely on kmalloc(PAGE_SIZE) returning PAGE_SIZE aligned memory
>> (Xen requires GDT *and* LDT to be page-aligned).
>
>Can kmalloc return non-page-aligned PAGE_SIZE allocations?
Documentation says it's to return pointer-size aligned memory - any excess
alignment is therefore an implementation detail. (Nevertheless, afaics all
current allocators generate page-aligned chunks.)
>> Using the page
>> allocator interface also removes the (albeit small) slab allocator
>> overhead.
>
>Runtime or space overhead? Given that they're once-off allocations, the
>time part isn't a big factor. And apparently LDT is completely unused.
Both, but as I also said the saving is small.
Jan
* Jan Beulich <[email protected]> wrote:
> Don't rely on kmalloc(PAGE_SIZE) returning PAGE_SIZE aligned memory
> (Xen requires GDT *and* LDT to be page-aligned). Using the page
> allocator interface also removes the (albeit small) slab allocator
> overhead. The same change being done for 64-bits for consistency.
>
> Further, the Xen hypercall interface expects the LDT address to be
> virtual, not machine.
coul dyou try this against x86.git as well? We already unified ldt.c as
part of the paravirt patches.
Ingo
>>> Ingo Molnar <[email protected]> 14.01.08 16:48 >>>
>
>* Jan Beulich <[email protected]> wrote:
>
>> Don't rely on kmalloc(PAGE_SIZE) returning PAGE_SIZE aligned memory
>> (Xen requires GDT *and* LDT to be page-aligned). Using the page
>> allocator interface also removes the (albeit small) slab allocator
>> overhead. The same change being done for 64-bits for consistency.
>>
>> Further, the Xen hypercall interface expects the LDT address to be
>> virtual, not machine.
>
>coul dyou try this against x86.git as well? We already unified ldt.c as
>part of the paravirt patches.
Sure, but it may take me a little while.
Jan
Jan Beulich wrote:
>>>> Ingo Molnar <[email protected]> 14.01.08 16:48 >>>
>>>>
>> * Jan Beulich <[email protected]> wrote:
>>
>>
>>> Don't rely on kmalloc(PAGE_SIZE) returning PAGE_SIZE aligned memory
>>> (Xen requires GDT *and* LDT to be page-aligned). Using the page
>>> allocator interface also removes the (albeit small) slab allocator
>>> overhead. The same change being done for 64-bits for consistency.
>>>
>>> Further, the Xen hypercall interface expects the LDT address to be
>>> virtual, not machine.
>>>
>> coul dyou try this against x86.git as well? We already unified ldt.c as
>> part of the paravirt patches.
>>
>
> Sure, but it may take me a little while.
I've got the unified version here.
Subject: xen: adjust/fix LDT handling for Xen
From: "Jan Beulich" <[email protected]>
Don't rely on kmalloc(PAGE_SIZE) returning PAGE_SIZE aligned memory
(Xen requires GDT *and* LDT to be page-aligned). Using the page
allocator interface also removes the (albeit small) slab allocator
overhead. The same change being done for 64-bits for consistency.
Further, the Xen hypercall interface expects the LDT address to be
virtual, not machine.
[ Adjusted to unified ldt.c - Jeremy ]
Signed-off-by: Jan Beulich <[email protected]>
Acked-by: Jeremy Fitzhardinge <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
arch/x86/kernel/ldt.c | 7 +++----
arch/x86/xen/enlighten.c | 9 +--------
2 files changed, 4 insertions(+), 12 deletions(-)
===================================================================
--- a/arch/x86/kernel/ldt.c
+++ b/arch/x86/kernel/ldt.c
@@ -12,7 +12,6 @@
#include <linux/mm.h>
#include <linux/smp.h>
#include <linux/vmalloc.h>
-#include <linux/slab.h>
#include <asm/uaccess.h>
#include <asm/system.h>
@@ -40,7 +39,7 @@ static int alloc_ldt(mm_context_t *pc, i
if (mincount * LDT_ENTRY_SIZE > PAGE_SIZE)
newldt = vmalloc(mincount * LDT_ENTRY_SIZE);
else
- newldt = kmalloc(mincount * LDT_ENTRY_SIZE, GFP_KERNEL);
+ newldt = (void *)__get_free_page(GFP_KERNEL);
if (!newldt)
return -ENOMEM;
@@ -78,7 +77,7 @@ static int alloc_ldt(mm_context_t *pc, i
if (oldsize * LDT_ENTRY_SIZE > PAGE_SIZE)
vfree(oldldt);
else
- kfree(oldldt);
+ put_page(virt_to_page(oldldt));
}
return 0;
}
@@ -129,7 +128,7 @@ void destroy_context(struct mm_struct *m
if (mm->context.size * LDT_ENTRY_SIZE > PAGE_SIZE)
vfree(mm->context.ldt);
else
- kfree(mm->context.ldt);
+ put_page(virt_to_page(mm->context.ldt));
mm->context.size = 0;
}
}
===================================================================
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -275,19 +275,12 @@ static unsigned long xen_store_tr(void)
static void xen_set_ldt(const void *addr, unsigned entries)
{
- unsigned long linear_addr = (unsigned long)addr;
struct mmuext_op *op;
struct multicall_space mcs = xen_mc_entry(sizeof(*op));
op = mcs.args;
op->cmd = MMUEXT_SET_LDT;
- if (linear_addr) {
- /* ldt my be vmalloced, use arbitrary_virt_to_machine */
- xmaddr_t maddr;
- maddr = arbitrary_virt_to_machine((unsigned long)addr);
- linear_addr = (unsigned long)maddr.maddr;
- }
- op->arg1.linear_addr = linear_addr;
+ op->arg1.linear_addr = (unsigned long)addr;
op->arg2.nr_ents = entries;
MULTI_mmuext_op(mcs.mc, op, 1, NULL, DOMID_SELF);
* Jeremy Fitzhardinge <[email protected]> wrote:
>>> coul dyou try this against x86.git as well? We already unified ldt.c as
>>> part of the paravirt patches.
>>>
>>
>> Sure, but it may take me a little while.
>
> I've got the unified version here.
thanks, applied.
> Acked-by: Jeremy Fitzhardinge <[email protected]>
please use Signed-off-by when passing through (or modifying) patches.
Ingo
Ingo Molnar wrote:
> * Jeremy Fitzhardinge <[email protected]> wrote:
>
>
>>>> coul dyou try this against x86.git as well? We already unified ldt.c as
>>>> part of the paravirt patches.
>>>>
>>>>
>>> Sure, but it may take me a little while.
>>>
>> I've got the unified version here.
>>
>
> thanks, applied.
>
>
>> Acked-by: Jeremy Fitzhardinge <[email protected]>
>>
>
> please use Signed-off-by when passing through (or modifying) patches.
>
Will do. That was left over from trying to apply the original version.
J