on kernel_eflags

While were looking into ret_from_fork code I somehow wondered about the global kernel_eflags variable here.

arch/x86/cpu/common.c
---------------------
unsigned long kernel_eflags;

void __cpuinit cpu_init(void)

{
	...
	raw_local_save_flags(kernel_eflags);
}

arch/x86/kernel/entry_64.S
--------------------------
ENTRY(ret_from_fork)

	DEFAULT_FRAME
	LOCK ; btr $TIF_FORK,TI_flags(%r8)
	pushq_cfi kernel_eflags(%rip)
	popfq_cfi                               # reset kernel eflags

Every call to cpu_init renew global kernel_eflags and every task switching does use this variable in a sake of cleaning carry bit of flags register as far as I can tell.

Should not every cpu has own copy of kernel_eflags? Just to be consistent in style? Or this would be space waisting and an optimization is done here?

I posted http://lkml.org/lkml/2010/2/9/155 to switch to the hardcoded version for 64 bit too. Is it worth my rebasing and reposting that?

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]

Quoted 19 lines · Ian Campbell

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.

Quoted 2 lines

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

Cc [email protected]

The whole series looks good to me, thanks Ian! I hope I don't miss anything, so lets wait for more feedback ;)

From a450bf9e1c6e3f436c652b3e4d63b50c1f9848d4 Mon Sep 17 00:00:00 2001

Quoted 3 lines

The whole series looks good to me, thanks Ian! I hope I don't miss anything, so lets wait for more feedback ;)

Thanks. Actually I missed the extern declaration in asm/processor.h so this updated patch should be used instead:

On Wed, Jul 06, 2011 at 10:25:29AM +0100, Ian Campbell wrote:
...

Quoted 7 lines

Quoted 3 lines

The whole series looks good to me, thanks Ian! I hope I don't miss anything, so lets wait for more feedback ;)

Thanks. Actually I missed the extern declaration in asm/processor.h so this updated patch should be used instead:

Great, thanks Ian. Have you a chance to run this series? ;)

Quoted 10 lines · Cyrill Gorcunov

On Wed, Jul 06, 2011 at 10:25:29AM +0100, Ian Campbell wrote:
...

Quoted 7 lines

Quoted 3 lines

The whole series looks good to me, thanks Ian! I hope I don't miss anything, so lets wait for more feedback ;)

Thanks. Actually I missed the extern declaration in asm/processor.h so this updated patch should be used instead:

Quoted 1 line

Great, thanks Ian. Have you a chance to run this series? ;)

Yeah I booted it before I posted it. I figured that the initscripts etc would be a sufficient test of fork() and that if it were wrong it would fall over pretty quickly and obviously.

Ian.

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(-)

arch/x86/kernel/cpu/common.c +0 -4

index 22a073d..178c3b4 100644
@@ -1077,8 +1077,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.

@@ -1231,8 +1229,6 @@ void __cpuinit cpu_init(void)
        fpu_init();
        xsave_init();
 
-       raw_local_save_flags(kernel_eflags);
-
        if (is_uv_system())
                uv_cpu_init();
  }

arch/x86/kernel/entry_64.S +1 -1

index e13329d..b50d142 100644
@@ -396,7 +396,7 @@ ENTRY(ret_from_fork)
 
        LOCK ; btr $TIF_FORK,TI_flags(%r8)
 
-       pushq_cfi kernel_eflags(%rip)
+       pushq_cfi $0x0002

       popfq_cfi                               # reset kernel eflags

       call schedule_tail                      # rdi: 'prev' task parameter

Well lets ask Linus and Andi?

Quoted 1 line

Well lets ask Linus and Andi?

The original idea was that if someone extends EFLAGS with new bits the BIOS could set them. Strictly said the Intel Manual specifies that ("reserved bits -- always set to values previously read")

But that never really worked out and I don't think anyone would dare extending it now. So hardcoding is fine I guess.

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]

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(-)

arch/x86/kernel/entry_32.S +0 -0

index 5c1a9197..f032530 100644
@@ -303,7 +303,7 @@ ENTRY(ret_from_fork)

       call schedule_tail
       GET_THREAD_INFO(%ebp)
       popl_cfi %eax
-       pushl_cfi $0x0202               # Reset kernel eflags
+       pushl_cfi $(X86_EFLAGS_IF|0x2)  # Reset kernel eflags popfl_cfi
       jmp syscall_exit
       CFI_ENDPROC

arch/x86/kernel/entry_64.S +0 -0

index 9c28bd5..4c76abf 100644
@@ -399,7 +399,7 @@ ENTRY(ret_from_fork)
 

       call schedule_tail                      # rdi: 'prev' task parameter

-       pushq_cfi $0x0202
+       pushq_cfi $(X86_EFLAGS_IF|0x2)
       popfq_cfi                               # reset kernel eflags

       GET_THREAD_INFO(%rcx)

Should not every cpu has own copy of kernel_eflags? Just to be consistent in style? Or this would be space waisting and an optimization is done here?

Quoted 5 lines · Cyrill Gorcunov

Should not every cpu has own copy of kernel_eflags? Just to be consistent in style? Or this would be space waisting and an optimization is done here?

Not specific to this particular case, but in general: a shared variable that used often but rarely written to will automatically replicate itself in the caches of multiple processors. This is the purpose of the read_mostly segment (writes are permitted but expected to be rare), which exists to make sure that a frequently written variable doesn't randomly end up in the cache line next to a read-mostly variable.

-hpa

Quoted 5 lines · Cyrill Gorcunov

Should not every cpu has own copy of kernel_eflags? Just to be consistent in style? Or this would be space waisting and an optimization is done here?

Not specific to this particular case, but in general: a shared variable that used often but rarely written to will automatically replicate itself in the caches of multiple processors. This is the purpose of the read_mostly segment (writes are permitted but expected to be rare), which exists to make sure that a frequently written variable doesn't randomly end up in the cache line next to a read-mostly variable.