All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@gmail.com>
To: Ian Campbell <Ian.Campbell@citrix.com>,
	linux-kernel@vger.kernel.org, Pekka Enberg <penberg@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>, Andi Kleen <ak@linux.intel.com>,
	Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH 0/3] minor cleanups to EFLAGS initialisation in ret_from_fork
Date: Mon, 25 Jul 2011 22:20:50 +0400	[thread overview]
Message-ID: <20110725182049.GD27137@sun> (raw)
In-Reply-To: <20110725101902.GP4362@sun>

On Mon, Jul 25, 2011 at 02:19:02PM +0400, Cyrill Gorcunov wrote:
> On Mon, Jul 25, 2011 at 10:58:03AM +0100, Ian Campbell wrote:
> > The following series removes the use of a global kernel_eflags variable
> > from the x86_64 ret_from_fork path and (very slightly) merges the 32 and
> > 64 bit version of that code path.
> > 
> > kernel_eflags could be made a __read_mostly but actually there is no
> > reason to prefer the value at cpu_init() time to a compile time constant
> > value for the initial eflags after a fork.
> > 
> > Ian.
> > 
> 
> Thanks, Ian! I think noone against this simplification, Peter, Andi?
> 
> 	Cyrill

Ian, I've missed in first place that you've opened IRQs window _before_
schedule_tail() call, ie it's not 1:1 code mapping as it was before.

Note kernel_eflags has IF clear and what we have: the ret_from_fork on
x86-64 happens _only_ inside context_switch call, ie

schedule (sched.c)
        ...
        raw_spin_lock_irq
        ...
        context_switch
                switch_to
                        "jnz   ret_from_fork\n\t"
                        pushq_cfi kernel_eflags(%rip)
                        popfq_cfi                               # reset kernel eflags

--->                    irqs are still disabled

                        call schedule_tail                      # rdi: 'prev' task parameter
                                finish_lock_switch
                                        raw_spin_unlock_irq

I bet raw_spin_lock_irq at the beginning of the schedule() is set
for a reason and such change is not safe. Though I may be missing
something again...

	Cyrill

  reply	other threads:[~2011-07-25 18:21 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-25  9:58 [PATCH 0/3] minor cleanups to EFLAGS initialisation in ret_from_fork Ian Campbell
2011-07-25 10:03 ` [PATCH 1/3] x86: drop unnecessary kernel_eflags variable from 64 bit Ian Campbell
2011-07-25 10:03 ` [PATCH 2/3] x86: make 64 bit ret_from_fork a little more similar to 32 bit Ian Campbell
2011-07-25 10:03 ` [PATCH 3/3] x86: ret_from_fork: use symbolic contants for bits in EFLAGS Ian Campbell
2011-07-25 10:19 ` [PATCH 0/3] minor cleanups to EFLAGS initialisation in ret_from_fork Cyrill Gorcunov
2011-07-25 18:20   ` Cyrill Gorcunov [this message]
2011-07-25 21:10     ` H. Peter Anvin
2011-07-25 21:47       ` Cyrill Gorcunov
2011-07-26 14:46         ` Peter Zijlstra
2011-07-26 15:51           ` Ian Campbell
2011-08-10 15:27           ` Ian Campbell
2011-08-10 15:35             ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110725182049.GD27137@sun \
    --to=gorcunov@gmail.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=ak@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=penberg@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.