|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCHv4] x86, fpu: remove the logic of non-eager fpu mem allocation at the first usage
On 05/03/15 22:08, H. Peter Anvin wrote:
> On 06/23/2014 06:08 AM, Konrad Rzeszutek Wilk wrote:
>> On Wed, Mar 19, 2014 at 08:02:22AM -0700, H. Peter Anvin wrote:
>>> On 03/19/2014 06:21 AM, Konrad Rzeszutek Wilk wrote:
>>>>>
>>>>> The following patch does the always eager allocation. It's a fixup of
>>>>> Suresh's original patch.
>>>>>
>>>>
>>>> Hey Peter,
>>>>
>>>> I think this is the solution you were looking for?
>>>>
>>>> Or are there some other subtle issues that you think lurk around?
>>>>
>>>
>>> Ah, I managed to miss it (mostly because it was buried *inside* another
>>> email and didn't change the subject line... I really dislike that mode
>>> of delivering a patch.
>>
>> Let me roll up some of these patchset and send them as git send-email.
>>
>>>
>>> Let me see if the issues have been fixed. Still wondering if there is a
>>> way we can get away without the boot_func hack...
>>
>> I have to confesss I don't even remember what the 'if the issues have been
>> fixed' is referring to?
>>
>
> Hi Konrad... it looks like this got left waiting for you and got forgotten?
8<--------------------------------
x86, fpu: remove the logic of non-eager fpu mem allocation at the first usage
For non-eager fpu mode, thread's fpu state is allocated during the first
fpu usage (in the context of device not available exception). This can be
a blocking call and hence we enable interrupts (which were originally
disabled when the exception happened), allocate memory and disable
interrupts etc. While this saves 512 bytes or so per-thread, there
are some issues in general.
a. Most of the future cases will be anyway using eager
FPU (because of processor features like xsaveopt, LWP, MPX etc) and
they do the allocation at the thread creation itself. Nice to have
one common mechanism as all the state save/restore code is
shared. Avoids the confusion and minimizes the subtle bugs
in the core piece involved with context-switch.
b. If a parent thread uses FPU, during fork() we allocate
the FPU state in the child and copy the state etc. Shortly after this,
during exec() we free it up, so that we can later allocate during
the first usage of FPU. So this free/allocate might be slower
for some workloads.
c. math_state_restore() is called from multiple places
and it is error pone if the caller expects interrupts to be disabled
throughout the execution of math_state_restore(). Can lead to subtle
bugs like Ubuntu bug #1265841.
Memory savings will be small anyways and the code complexity
introducing subtle bugs is not worth it. So remove
the logic of non-eager fpu mem allocation at the first usage.
Signed-off-by: Suresh Siddha <sbsiddha@xxxxxxxxx>
Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
---
v4:
- add __ref to fpu_init() which needs to allocate the fpu state memory
for the first task on the boot cpu.
v3:
- Rebase on 3.17-rc3.
v2:
- Tweak condition for allocating the first thread's FPU state.
---
arch/x86/kernel/i387.c | 20 +++++++++++---------
arch/x86/kernel/process.c | 6 ------
arch/x86/kernel/traps.c | 16 ++--------------
arch/x86/kernel/xsave.c | 2 --
4 files changed, 13 insertions(+), 31 deletions(-)
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index d5651fc..089defc 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -5,6 +5,7 @@
* General FPU state handling cleanups
* Gareth Hughes <gareth@xxxxxxxxxxx>, May 2000
*/
+#include <linux/bootmem.h>
#include <linux/module.h>
#include <linux/regset.h>
#include <linux/sched.h>
@@ -211,6 +212,16 @@ void fpu_init(void)
mxcsr_feature_mask_init();
xsave_init();
+
+ /*
+ * Now we know the final size of the xsave data, allocate the
+ * FPU state area for the first task. All other tasks have
+ * this allocated in arch_dup_task_struct().
+ */
+ if (!current->thread.fpu.state)
+ current->thread.fpu.state = alloc_bootmem_align(xstate_size,
+ __alignof__(struct xsave_struct));
+
eager_fpu_init();
}
@@ -242,8 +253,6 @@ EXPORT_SYMBOL_GPL(fpu_finit);
*/
int init_fpu(struct task_struct *tsk)
{
- int ret;
-
if (tsk_used_math(tsk)) {
if (cpu_has_fpu && tsk == current)
unlazy_fpu(tsk);
@@ -251,13 +260,6 @@ int init_fpu(struct task_struct *tsk)
return 0;
}
- /*
- * Memory allocation at the first usage of the FPU and other state.
- */
- ret = fpu_alloc(&tsk->thread.fpu);
- if (ret)
- return ret;
-
fpu_finit(&tsk->thread.fpu);
set_stopped_child_used_math(tsk);
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 046e2d6..5f4e746 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -132,12 +132,6 @@ void flush_thread(void)
flush_ptrace_hw_breakpoint(tsk);
memset(tsk->thread.tls_array, 0, sizeof(tsk->thread.tls_array));
drop_init_fpu(tsk);
- /*
- * Free the FPU state for non xsave platforms. They get reallocated
- * lazily at the first use.
- */
- if (!use_eager_fpu())
- free_thread_xstate(tsk);
}
static void hard_disable_TSC(void)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 9d2073e..dbbf5e0 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -844,20 +844,8 @@ void math_state_restore(void)
{
struct task_struct *tsk = current;
- if (!tsk_used_math(tsk)) {
- local_irq_enable();
- /*
- * does a slab alloc which can sleep
- */
- if (init_fpu(tsk)) {
- /*
- * ran out of memory!
- */
- do_group_exit(SIGKILL);
- return;
- }
- local_irq_disable();
- }
+ if (!tsk_used_math(tsk))
+ init_fpu(tsk);
/* Avoid __kernel_fpu_begin() right after __thread_fpu_begin() */
kernel_fpu_disable();
diff --git a/arch/x86/kernel/xsave.c b/arch/x86/kernel/xsave.c
index 34f66e5..5e21dd9 100644
--- a/arch/x86/kernel/xsave.c
+++ b/arch/x86/kernel/xsave.c
@@ -679,8 +679,6 @@ void xsave_init(void)
static inline void __init eager_fpu_init_bp(void)
{
- current->thread.fpu.state =
- alloc_bootmem_align(xstate_size, __alignof__(struct xsave_struct));
if (!init_xstate_buf)
setup_init_fpu_buf();
}
--
1.7.10.4
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |