2010-02-09 15:56:19

by Ian Campbell

[permalink] [raw]
Subject: [PATCH 0/3] x86: slightly unify ret_from_fork

I came across some minor differences between the 32 and 64 bit
implementations of ret_from_fork. Although I don't expect this is code
which can realistically be completely unified there seemed to be some
value in making the two look a bit more similar.

Signed-off-by: Ian Campbell <[email protected]>
Cc: [email protected]


2010-02-09 15:56:20

by Ian Campbell

[permalink] [raw]
Subject: [PATCH 2/3] x86: make 64 bit ret_from_fork a little more similar to 32 bit

The 64 bit version resets EFLAGS before calling schedule_tail() and
therefore leaves EFLAGS.IF clear. 32 bit resets EFLAGS after calling
schedule_tail() and therefore leaves EFLAGS.IF set. I don't think
there is any practical difference between the two approaches since
interrupts are actually reenabled within schedule_tail
(schedule_tail->finish_task_switch->finish_lock_switch->raw_spin_unlock_irq->...->local_irq_enable)
so arbitrarily pick the 32 bit version and make 64 bit look like that.

Signed-off-by: Ian Campbell <[email protected]>
Cc: [email protected]
---
arch/x86/kernel/entry_64.S | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 18e4c59..b771c2f 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -398,13 +398,13 @@ ENTRY(ret_from_fork)

LOCK ; btr $TIF_FORK,TI_flags(%r8)

- push $0x0002
+ call schedule_tail # rdi: 'prev' task parameter
+
+ push $0x0202
CFI_ADJUST_CFA_OFFSET 8
popf # reset kernel eflags
CFI_ADJUST_CFA_OFFSET -8

- call schedule_tail # rdi: 'prev' task parameter
-
GET_THREAD_INFO(%rcx)

RESTORE_REST
--
1.5.6.5

2010-02-09 15:56:33

by Ian Campbell

[permalink] [raw]
Subject: [PATCH 3/3] x86: ret_from_fork: use symbolic contants for bits in EFLAGS

Signed-off-by: Ian Campbell <[email protected]>
Cc: [email protected]
---
arch/x86/kernel/entry_32.S | 2 +-
arch/x86/kernel/entry_64.S | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index 44a8e0d..02a5a18 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -325,7 +325,7 @@ ENTRY(ret_from_fork)
GET_THREAD_INFO(%ebp)
popl %eax
CFI_ADJUST_CFA_OFFSET -4
- pushl $0x0202 # Reset kernel eflags
+ pushl $(X86_EFLAGS_IF|0x2) # Reset kernel eflags
CFI_ADJUST_CFA_OFFSET 4
popfl
CFI_ADJUST_CFA_OFFSET -4
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index b771c2f..f5ceffa 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -400,7 +400,7 @@ ENTRY(ret_from_fork)

call schedule_tail # rdi: 'prev' task parameter

- push $0x0202
+ push $(X86_EFLAGS_IF|0x2)
CFI_ADJUST_CFA_OFFSET 8
popf # reset kernel eflags
CFI_ADJUST_CFA_OFFSET -8
--
1.5.6.5

2010-02-09 15:56:45

by Ian Campbell

[permalink] [raw]
Subject: [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit.

For no reason that I can determine 64 bit x86 saves the current eflags
in cpu_init purely for use in ret_from_fork. The equivalent 32 bit
code simply hard codes 0x0202 as the new EFLAGS which seems safer than
relying on a potentially arbitrary EFLAGS saved during cpu_init.

Original i386 changeset
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=47a5c6fa0e204a2b63309c648bb2fde36836c826
Original x86_64 changset
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=658fdbef66e5e9be79b457edc2cbbb3add840aa9

The only comment in the later indicates that it is following the
former, but not why it differs in this way.

This change makes 64 bit use the same mechanism to setup the initial
EFLAGS on fork. Note that 64 bit resets EFLAGS before calling
schedule_tail() as opposed to 32 bit which calls schedule_tail()
first. Therefore the correct value for EFLAGS has opposite IF
bit. This will be fixed in a subsequent patch.

Signed-off-by: Ian Campbell <[email protected]>
Cc: [email protected]
---
arch/x86/kernel/cpu/common.c | 4 ----
arch/x86/kernel/entry_64.S | 2 +-
2 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 4868e4a..ab766b2 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1040,8 +1040,6 @@ void syscall_init(void)
X86_EFLAGS_TF|X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_IOPL);
}

-unsigned long kernel_eflags;
-
/*
* Copies of the original ist values from the tss are only accessed during
* debugging, no special alignment required.
@@ -1189,8 +1187,6 @@ void __cpuinit cpu_init(void)

fpu_init();

- raw_local_save_flags(kernel_eflags);
-
if (is_uv_system())
uv_cpu_init();
}
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 0697ff1..18e4c59 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -398,7 +398,7 @@ ENTRY(ret_from_fork)

LOCK ; btr $TIF_FORK,TI_flags(%r8)

- push kernel_eflags(%rip)
+ push $0x0002
CFI_ADJUST_CFA_OFFSET 8
popf # reset kernel eflags
CFI_ADJUST_CFA_OFFSET -8
--
1.5.6.5