Only allocate the FPU area when the application actually uses FPU, i.e., in the
first lazy FPU trap. This could save memory for non-fpu using apps.
Signed-off-by: Suresh Siddha <[email protected]>
Cc: Arjan van de Ven <[email protected]>
---
v3: Fixed the non-atomic calling sequence in atomic context.
v2: Ported to x86.git#testing with some name changes.
---
Index: linux-2.6-x86/arch/x86/kernel/i387.c
===================================================================
--- linux-2.6-x86.orig/arch/x86/kernel/i387.c 2008-03-03 14:15:17.000000000 -0800
+++ linux-2.6-x86/arch/x86/kernel/i387.c 2008-03-03 14:15:17.000000000 -0800
@@ -9,7 +9,6 @@
#include <linux/sched.h>
#include <linux/module.h>
#include <linux/regset.h>
-#include <linux/bootmem.h>
#include <asm/processor.h>
#include <asm/i387.h>
#include <asm/math_emu.h>
@@ -67,7 +66,6 @@
else
xstate_size = sizeof(struct i387_fsave_struct);
#endif
- init_task.thread.xstate = alloc_bootmem(xstate_size);
}
#ifdef CONFIG_X86_64
@@ -105,6 +103,12 @@
return;
}
+ /*
+ * Memory allocation at the first usage of the FPU and other state.
+ */
+ if (!tsk->thread.xstate)
+ tsk->thread.xstate = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
+
if (cpu_has_fxsr) {
struct i387_fxsave_struct *fx = &tsk->thread.xstate->fxsave;
Index: linux-2.6-x86/arch/x86/kernel/process.c
===================================================================
--- linux-2.6-x86.orig/arch/x86/kernel/process.c 2008-03-03 14:15:17.000000000 -0800
+++ linux-2.6-x86/arch/x86/kernel/process.c 2008-03-03 14:15:17.000000000 -0800
@@ -5,24 +5,33 @@
#include <linux/slab.h>
#include <linux/sched.h>
-static struct kmem_cache *task_xstate_cachep;
+struct kmem_cache *task_xstate_cachep;
int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
{
*dst = *src;
- dst->thread.xstate = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
- if (!dst->thread.xstate)
- return -ENOMEM;
- WARN_ON((unsigned long)dst->thread.xstate & 15);
- memcpy(dst->thread.xstate, src->thread.xstate, xstate_size);
+ if (src->thread.xstate) {
+ dst->thread.xstate = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
+ if (!dst->thread.xstate)
+ return -ENOMEM;
+ WARN_ON((unsigned long)dst->thread.xstate & 15);
+ memcpy(dst->thread.xstate, src->thread.xstate, xstate_size);
+ }
return 0;
}
-void free_thread_info(struct thread_info *ti)
+void free_thread_xstate(struct task_struct *tsk)
{
- kmem_cache_free(task_xstate_cachep, ti->task->thread.xstate);
- ti->task->thread.xstate = NULL;
+ if (tsk->thread.xstate) {
+ kmem_cache_free(task_xstate_cachep, tsk->thread.xstate);
+ tsk->thread.xstate = NULL;
+ }
+}
+
+void free_thread_info(struct thread_info *ti)
+{
+ free_thread_xstate(ti->task);
free_pages((unsigned long)(ti), get_order(THREAD_SIZE));
}
Index: linux-2.6-x86/include/asm-x86/processor.h
===================================================================
--- linux-2.6-x86.orig/include/asm-x86/processor.h 2008-03-03 14:15:17.000000000 -0800
+++ linux-2.6-x86/include/asm-x86/processor.h 2008-03-03 14:15:17.000000000 -0800
@@ -354,6 +354,8 @@
extern void print_cpu_info(struct cpuinfo_x86 *);
extern unsigned int xstate_size;
+extern void free_thread_xstate(struct task_struct *);
+extern struct kmem_cache *task_xstate_cachep;
extern void init_scattered_cpuid_features(struct cpuinfo_x86 *c);
extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
extern unsigned short num_cache_leaves;
Index: linux-2.6-x86/arch/x86/kernel/process_32.c
===================================================================
--- linux-2.6-x86.orig/arch/x86/kernel/process_32.c 2008-03-03 14:15:17.000000000 -0800
+++ linux-2.6-x86/arch/x86/kernel/process_32.c 2008-03-03 14:15:17.000000000 -0800
@@ -524,6 +524,10 @@
regs->cs = __USER_CS;
regs->ip = new_ip;
regs->sp = new_sp;
+ /*
+ * Free the old FP and other extended state
+ */
+ free_thread_xstate(current);
}
EXPORT_SYMBOL_GPL(start_thread);
Index: linux-2.6-x86/arch/x86/kernel/process_64.c
===================================================================
--- linux-2.6-x86.orig/arch/x86/kernel/process_64.c 2008-03-03 14:15:17.000000000 -0800
+++ linux-2.6-x86/arch/x86/kernel/process_64.c 2008-03-03 14:15:17.000000000 -0800
@@ -552,6 +552,10 @@
regs->ss = __USER_DS;
regs->flags = 0x200;
set_fs(USER_DS);
+ /*
+ * Free the old FP and other extended state
+ */
+ free_thread_xstate(current);
}
EXPORT_SYMBOL_GPL(start_thread);
Index: linux-2.6-x86/arch/x86/kernel/traps_32.c
===================================================================
--- linux-2.6-x86.orig/arch/x86/kernel/traps_32.c 2008-03-03 14:15:17.000000000 -0800
+++ linux-2.6-x86/arch/x86/kernel/traps_32.c 2008-03-03 14:17:28.000000000 -0800
@@ -1169,9 +1169,20 @@
struct thread_info *thread = current_thread_info();
struct task_struct *tsk = thread->task;
- clts(); /* Allow maths ops (or we recurse) */
- if (!tsk_used_math(tsk))
+ if (!tsk_used_math(tsk)) {
+#ifdef CONFIG_PREEMPT
+ local_irq_enable();
+#endif
+ /*
+ * does a slab alloc which can sleep
+ */
init_fpu(tsk);
+#ifdef CONFIG_PREEMPT
+ local_irq_disable();
+#endif
+ }
+
+ clts(); /* Allow maths ops (or we recurse) */
restore_fpu(tsk);
thread->status |= TS_USEDFPU; /* So we fnsave on switch_to() */
tsk->fpu_counter++;
--
On Mon, Mar 03, 2008 at 03:02:46PM -0800, Suresh Siddha wrote:
> + /*
> + * Memory allocation at the first usage of the FPU and other state.
> + */
> + if (!tsk->thread.xstate)
> + tsk->thread.xstate = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
Please don't do over 80 char lines. Also don't we need some kind of
error handling here?
On Mon, Mar 03, 2008 at 08:20:12PM -0500, Christoph Hellwig wrote:
> On Mon, Mar 03, 2008 at 03:02:46PM -0800, Suresh Siddha wrote:
> > + /*
> > + * Memory allocation at the first usage of the FPU and other state.
> > + */
> > + if (!tsk->thread.xstate)
> > + tsk->thread.xstate = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
>
> Please don't do over 80 char lines.
Ok.
> Also don't we need some kind of error handling here?
Currently it uses SLAB_PANIC.
thanks,
suresh
* Suresh Siddha <[email protected]> wrote:
> On Mon, Mar 03, 2008 at 08:20:12PM -0500, Christoph Hellwig wrote:
> > On Mon, Mar 03, 2008 at 03:02:46PM -0800, Suresh Siddha wrote:
> > > + /*
> > > + * Memory allocation at the first usage of the FPU and other state.
> > > + */
> > > + if (!tsk->thread.xstate)
> > > + tsk->thread.xstate = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
> >
> > Also don't we need some kind of error handling here?
>
> Currently it uses SLAB_PANIC.
but SLAB_PANIC only covers kmem_cache_create() failures.
kmem_cache_alloc() can fail (return NULL) and not handling it is a bug.
Ingo
On Tue, Mar 04, 2008 at 11:32:20AM +0100, Ingo Molnar wrote:
>
> * Suresh Siddha <[email protected]> wrote:
>
> > On Mon, Mar 03, 2008 at 08:20:12PM -0500, Christoph Hellwig wrote:
> > > On Mon, Mar 03, 2008 at 03:02:46PM -0800, Suresh Siddha wrote:
> > > > + /*
> > > > + * Memory allocation at the first usage of the FPU and other state.
> > > > + */
> > > > + if (!tsk->thread.xstate)
> > > > + tsk->thread.xstate = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
> > >
> > > Also don't we need some kind of error handling here?
> >
> > Currently it uses SLAB_PANIC.
>
> but SLAB_PANIC only covers kmem_cache_create() failures.
>
> kmem_cache_alloc() can fail (return NULL) and not handling it is a bug.
oops. you are correct. Will send a sigsegv in the failure case then. Thanks.
On Mon 2008-03-03 15:02:46, Suresh Siddha wrote:
> Only allocate the FPU area when the application actually uses FPU, i.e., in the
> first lazy FPU trap. This could save memory for non-fpu using apps.
How many such apps are on your system, and how much does this
'optimalization' cost?
ISTR glibc always using FPU...?
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Tue 2008-03-04 09:55:28, Suresh Siddha wrote:
> On Tue, Mar 04, 2008 at 11:32:20AM +0100, Ingo Molnar wrote:
> >
> > * Suresh Siddha <[email protected]> wrote:
> >
> > > On Mon, Mar 03, 2008 at 08:20:12PM -0500, Christoph Hellwig wrote:
> > > > On Mon, Mar 03, 2008 at 03:02:46PM -0800, Suresh Siddha wrote:
> > > > > + /*
> > > > > + * Memory allocation at the first usage of the FPU and other state.
> > > > > + */
> > > > > + if (!tsk->thread.xstate)
> > > > > + tsk->thread.xstate = kmem_cache_alloc(task_xstate_cachep, GFP_KERNEL);
> > > >
> > > > Also don't we need some kind of error handling here?
> > >
> > > Currently it uses SLAB_PANIC.
> >
> > but SLAB_PANIC only covers kmem_cache_create() failures.
> >
> > kmem_cache_alloc() can fail (return NULL) and not handling it is a bug.
>
> oops. you are correct. Will send a sigsegv in the failure case then. Thanks.
You are introducing possibility of hard to debug error, where previous
code just worked... Does not look like good idea to me.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
* Pavel Machek <[email protected]> wrote:
> > > kmem_cache_alloc() can fail (return NULL) and not handling it is a
> > > bug.
> >
> > oops. you are correct. Will send a sigsegv in the failure case then.
> > Thanks.
>
> You are introducing possibility of hard to debug error, where previous
> code just worked... Does not look like good idea to me.
hm, how does it differ from any other allocation failure? We could fail
to allocate a pagetable page. We could fail to allocate the task_struct
to begin with.
Ingo
On Thu, Mar 06, 2008 at 04:51:41PM +0100, Ingo Molnar wrote:
>
> * Pavel Machek <[email protected]> wrote:
>
> > > > kmem_cache_alloc() can fail (return NULL) and not handling it is a
> > > > bug.
> > >
> > > oops. you are correct. Will send a sigsegv in the failure case then.
> > > Thanks.
> >
> > You are introducing possibility of hard to debug error, where previous
> > code just worked... Does not look like good idea to me.
>
> hm, how does it differ from any other allocation failure? We could fail
> to allocate a pagetable page. We could fail to allocate the task_struct
> to begin with.
Yes. This happens under out of memory conditions. And we are also using
GFP_KERNEL here, which can block.
On Wed, Mar 05, 2008 at 08:48:01PM +0100, Pavel Machek wrote:
> On Mon 2008-03-03 15:02:46, Suresh Siddha wrote:
> > Only allocate the FPU area when the application actually uses FPU, i.e., in the
> > first lazy FPU trap. This could save memory for non-fpu using apps.
>
> How many such apps are on your system, and how much does this
> 'optimalization' cost?
On a normal kernel boot, where there are 200 or so tasks running, only 20
or so apps seem to be using FPU.
> ISTR glibc always using FPU...?
Apparently not, alteast not on my system. And also going forward, as we extend
this state, thought is nice to have.
thanks,
suresh
On Thu 2008-03-06 16:51:41, Ingo Molnar wrote:
>
> * Pavel Machek <[email protected]> wrote:
>
> > > > kmem_cache_alloc() can fail (return NULL) and not handling it is a
> > > > bug.
> > >
> > > oops. you are correct. Will send a sigsegv in the failure case then.
> > > Thanks.
> >
> > You are introducing possibility of hard to debug error, where previous
> > code just worked... Does not look like good idea to me.
>
> hm, how does it differ from any other allocation failure? We could fail
Well, we should not be sending SIGSEGV...? SIGBUS would be cleaner, or
SIGKILL... what happens when userland tries to catch this one?
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Pavel Machek <[email protected]> writes:
>
> Well, we should not be sending SIGSEGV...? SIGBUS would be cleaner, or
> SIGKILL... what happens when userland tries to catch this one?
When this happens the kernel is already in a severe out of memory
situation and no matter what you do user land will not be able
to handle this well.
-Andi
On Thu 2008-03-06 11:26:53, Suresh Siddha wrote:
> On Wed, Mar 05, 2008 at 08:48:01PM +0100, Pavel Machek wrote:
> > On Mon 2008-03-03 15:02:46, Suresh Siddha wrote:
> > > Only allocate the FPU area when the application actually uses FPU, i.e., in the
> > > first lazy FPU trap. This could save memory for non-fpu using apps.
> >
> > How many such apps are on your system, and how much does this
> > 'optimalization' cost?
>
> On a normal kernel boot, where there are 200 or so tasks running, only 20
> or so apps seem to be using FPU.
Aha, now I see it is useful ;-).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Pavel Machek wrote:
> On Thu 2008-03-06 16:51:41, Ingo Molnar wrote:
>> * Pavel Machek <[email protected]> wrote:
>>
>>>>> kmem_cache_alloc() can fail (return NULL) and not handling it is a
>>>>> bug.
>>>> oops. you are correct. Will send a sigsegv in the failure case then.
>>>> Thanks.
>>> You are introducing possibility of hard to debug error, where previous
>>> code just worked... Does not look like good idea to me.
>> hm, how does it differ from any other allocation failure? We could fail
>
> Well, we should not be sending SIGSEGV...? SIGBUS would be cleaner, or
> SIGKILL... what happens when userland tries to catch this one?
>
I'm confused...
Normally when we need memory for userspace and can't get it, we put the
process to sleep until memory is available.
Why is this different in any way?
-hpa
H. Peter Anvin wrote:
> Pavel Machek wrote:
>> On Thu 2008-03-06 16:51:41, Ingo Molnar wrote:
>>> * Pavel Machek <[email protected]> wrote:
>>>
>>>>>> kmem_cache_alloc() can fail (return NULL) and not handling it is a
>>>>>> bug.
>>>>> oops. you are correct. Will send a sigsegv in the failure case
>>>>> then. Thanks.
>>>> You are introducing possibility of hard to debug error, where
>>>> previous code just worked... Does not look like good idea to me.
>>> hm, how does it differ from any other allocation failure? We could fail
>>
>> Well, we should not be sending SIGSEGV...? SIGBUS would be cleaner, or
>> SIGKILL... what happens when userland tries to catch this one?
>>
>
> I'm confused...
>
> Normally when we need memory for userspace and can't get it, we put the
> process to sleep until memory is available.
that's what GFP_KERNEL does
>
> Why is this different in any way?
this is just for handling the case where that fails
(basically near/totally OOM or the case where you get a fatal signal)
maybe we need a GFP_KILLABLE now that we have a TASK_KILLABLE...
> this is just for handling the case where that fails
> (basically near/totally OOM or the case where you get a fatal signal)
I didn't think GFP_KERNEL was interruptible by signals...
(although sometimes under oom thrashing I think it would be great if it was...)
-Andi
Andi Kleen wrote:
>> this is just for handling the case where that fails
>> (basically near/totally OOM or the case where you get a fatal signal)
>
> I didn't think GFP_KERNEL was interruptible by signals...
> (although sometimes under oom thrashing I think it would be great if it was...)
we need to make it (or with GFP_KILLABLE); would make total sense...
(so yeah it was more wishful thinking than reality)
On Fri, Mar 07, 2008 at 07:20:44AM -0600, Arjan van de Ven wrote:
> Andi Kleen wrote:
> >>this is just for handling the case where that fails
> >>(basically near/totally OOM or the case where you get a fatal signal)
> >
> >I didn't think GFP_KERNEL was interruptible by signals...
> >(although sometimes under oom thrashing I think it would be great if it
> >was...)
>
> we need to make it (or with GFP_KILLABLE); would make total sense...
> (so yeah it was more wishful thinking than reality)
I think it wouldn't be that difficult for the normal anonymous user allocations
(standard page fault path), but doing it for everything would be pretty
hard because you would need to add signal-bail-out paths everywhere.
But doing it for some simple cases like page fault only would be a nice
project for someone, shouldn't be too difficult.
-Andi