LKML Archive on lore.kernel.org
 help / color / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Andrew Lutomirski <luto@kernel.org>,
	Ingo Molnar <mingo@kernel.org>, Kees Cook <keescook@chromium.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Oleg Nesterov <oleg@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Denys Vlasenko <vda.linux@googlemail.com>,
	Rik van Riel <riel@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	Brian Gerst <brgerst@gmail.com>,
	"linux-tip-commits@vger.kernel.org" 
	<linux-tip-commits@vger.kernel.org>
Subject: Re: [tip:x86/asm] x86/irq, context_tracking: Document how IRQ context tracking works and add an RCU assertion
Date: Sat, 18 Jul 2015 07:10:13 -0700
Message-ID: <20150718141013.GS3717@linux.vnet.ibm.com> (raw)
In-Reply-To: <20150718132357.GC1747@lerouge>

On Sat, Jul 18, 2015 at 03:23:57PM +0200, Frederic Weisbecker wrote:
> On Tue, Jul 14, 2015 at 04:33:39PM -0700, Andy Lutomirski wrote:
> > On Tue, Jul 14, 2015 at 4:26 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > On Tue, Jul 07, 2015 at 03:54:32AM -0700, tip-bot for Andy Lutomirski wrote:
> > >> Commit-ID:  0333a209cbf600e980fc55c24878a56f25f48b65
> > >> Gitweb:     http://git.kernel.org/tip/0333a209cbf600e980fc55c24878a56f25f48b65
> > >> Author:     Andy Lutomirski <luto@kernel.org>
> > >> AuthorDate: Fri, 3 Jul 2015 12:44:34 -0700
> > >> Committer:  Ingo Molnar <mingo@kernel.org>
> > >> CommitDate: Tue, 7 Jul 2015 10:59:10 +0200
> > >>
> > >> x86/irq, context_tracking: Document how IRQ context tracking works and add an RCU assertion
> > >>
> > >> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > >> Cc: Andy Lutomirski <luto@amacapital.net>
> > >> Cc: Borislav Petkov <bp@alien8.de>
> > >> Cc: Brian Gerst <brgerst@gmail.com>
> > >> Cc: Denys Vlasenko <dvlasenk@redhat.com>
> > >> Cc: Denys Vlasenko <vda.linux@googlemail.com>
> > >> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > >> Cc: H. Peter Anvin <hpa@zytor.com>
> > >> Cc: Kees Cook <keescook@chromium.org>
> > >> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > >> Cc: Oleg Nesterov <oleg@redhat.com>
> > >> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > >> Cc: Peter Zijlstra <peterz@infradead.org>
> > >> Cc: Rik van Riel <riel@redhat.com>
> > >> Cc: Thomas Gleixner <tglx@linutronix.de>
> > >> Cc: paulmck@linux.vnet.ibm.com
> > >> Link: http://lkml.kernel.org/r/e8bdc4ed0193fb2fd130f3d6b7b8023e2ec1ab62.1435952415.git.luto@kernel.org
> > >> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > >> ---
> > >>  arch/x86/kernel/irq.c | 15 +++++++++++++++
> > >>  1 file changed, 15 insertions(+)
> > >>
> > >> diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
> > >> index 88b36648..6233de0 100644
> > >> --- a/arch/x86/kernel/irq.c
> > >> +++ b/arch/x86/kernel/irq.c
> > >> @@ -216,8 +216,23 @@ __visible unsigned int __irq_entry do_IRQ(struct pt_regs *regs)
> > >>       unsigned vector = ~regs->orig_ax;
> > >>       unsigned irq;
> > >>
> > >> +     /*
> > >> +      * NB: Unlike exception entries, IRQ entries do not reliably
> > >> +      * handle context tracking in the low-level entry code.  This is
> > >> +      * because syscall entries execute briefly with IRQs on before
> > >> +      * updating context tracking state, so we can take an IRQ from
> > >> +      * kernel mode with CONTEXT_USER.  The low-level entry code only
> > >> +      * updates the context if we came from user mode, so we won't
> > >> +      * switch to CONTEXT_KERNEL.  We'll fix that once the syscall
> > >> +      * code is cleaned up enough that we can cleanly defer enabling
> > >> +      * IRQs.
> > >> +      */
> > >> +
> > >
> > > Now is it a problem to take interrupts in kernel mode with CONTEXT_USER?
> > > I'm not sure it's worth trying to make it not happen.
> > 
> > It's not currently a problem, but it would be nice if we could do the
> > equivalent of:
> > 
> > if (user_mode(regs)) {
> >   user_exit();  (or enter_from_user_mode or whatever)
> > } else {
> >   // don't bother -- already in CONTEXT_KERNEL
> > }
> 
> This was the initial implementation of context tracking but it was terribly
> buggy. What if we enter the kernel, we haven't yet got a change to call
> context_tracking_user_exit() and we get an exception in the kernel entry
> path? user_mode(regs) will return the wrong value and bad things happen.
> 
> This is why context tracking needs its own tracking state, because we are always
> out of sync with the real processor context anyway.
> 
> > 
> > i.e. the same thing that do_general_protection, etc do in -tip.  That
> > would get rid of any need to store the previous context.
> > 
> > Currently we can't because of syscalls and maybe because of KVM.  KVM
> > has a weird fake interrupt thing.
> > 
> > >
> > >>       entering_irq();
> > >>
> > >> +     /* entering_irq() tells RCU that we're not quiescent.  Check it. */
> > >> +     rcu_lockdep_assert(rcu_is_watching(), "IRQ failed to wake up RCU");
> > >
> > > Why do we need to check that?
> > 
> > Sanity check.  If we're changing a bunch of context tracking details,
> > I want to assert that it actually works.
> 
> But we call rcu_irq_enter() right before.
> 
> It's more or less like doing:
> 
> local_irq_disable();
> WARN_ON(!irqs_disabled());

If we end up in a world where RCU sometimes uses context-tracking state
and sometimes uses its own state (for example, for architecture that
do not support context tracking), such a check might make more sense.
It would be all too easy for someone to accidentailly manage to disable
both somehow, and things would sort of work but have strange undebuggable
failure cases.  Sometimes.

							Thanx, Paul


  reply index

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-03 19:44 [PATCH v5 00/17] x86: Rewrite exit-to-userspace code Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 01/17] selftests/x86: Add a test for 32-bit fast syscall arg faults Andy Lutomirski
2015-07-07 10:49   ` [tip:x86/asm] x86/entry, " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 02/17] x86/entry/64/compat: Fix bad fast syscall arg failure path Andy Lutomirski
2015-07-07 10:49   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 03/17] uml: Fix do_signal() prototype Andy Lutomirski
2015-07-07 10:49   ` [tip:x86/asm] um: " tip-bot for Ingo Molnar
2015-07-03 19:44 ` [PATCH v5 04/17] context_tracking: Add ct_state and CT_WARN_ON Andy Lutomirski
2015-07-07 10:50   ` [tip:x86/asm] context_tracking: Add ct_state() and CT_WARN_ON() tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 05/17] notifiers: Assert that RCU is watching in notify_die Andy Lutomirski
2015-07-07 10:50   ` [tip:x86/asm] notifiers, RCU: Assert that RCU is watching in notify_die() tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 06/17] x86: Move C entry and exit code to arch/x86/entry/common.c Andy Lutomirski
2015-07-07 10:50   ` [tip:x86/asm] x86/entry: Move C entry and exit code to arch/x86/ entry/common.c tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 07/17] x86/traps: Assert that we're in CONTEXT_KERNEL in exception entries Andy Lutomirski
2015-07-07 10:51   ` [tip:x86/asm] x86/traps, context_tracking: Assert that we' re " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 08/17] x86/entry: Add enter_from_user_mode and use it in syscalls Andy Lutomirski
2015-07-07 10:51   ` [tip:x86/asm] x86/entry: Add enter_from_user_mode() " tip-bot for Andy Lutomirski
2015-07-14 23:00     ` Frederic Weisbecker
2015-07-14 23:04       ` Andy Lutomirski
2015-07-14 23:28         ` Frederic Weisbecker
2015-12-21 20:50   ` [PATCH v5 08/17] x86/entry: Add enter_from_user_mode " Sasha Levin
2015-12-21 22:44     ` Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 09/17] x86/entry: Add new, comprehensible entry and exit hooks Andy Lutomirski
2015-07-07 10:51   ` [tip:x86/asm] x86/entry: Add new, comprehensible entry and exit handlers written in C tip-bot for Andy Lutomirski
2015-07-14 23:07     ` Frederic Weisbecker
2015-07-15 19:56       ` Linus Torvalds
2015-07-15 20:46         ` Andy Lutomirski
2015-07-15 21:25           ` [PATCH] x86/entry: Fix _TIF_USER_RETURN_NOTIFY check in prepare_exit_to_usermode Andy Lutomirski
2015-07-18  3:25             ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 10/17] x86/entry/64: Really create an error-entry-from-usermode code path Andy Lutomirski
2015-07-07 10:52   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 11/17] x86/entry/64: Migrate 64-bit and compat syscalls to new exit hooks Andy Lutomirski
2015-07-07 10:52   ` [tip:x86/asm] x86/entry/64: Migrate 64-bit and compat syscalls to the new exit handlers and remove old assembly code tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 12/17] x86/asm/entry/64: Save all regs on interrupt entry Andy Lutomirski
2015-07-07 10:52   ` [tip:x86/asm] " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 13/17] x86/asm/entry/64: Simplify irq stack pt_regs handling Andy Lutomirski
2015-07-07 10:53   ` [tip:x86/asm] x86/asm/entry/64: Simplify IRQ " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 14/17] x86/asm/entry/64: Migrate error and interrupt exit work to C Andy Lutomirski
2015-07-07 10:53   ` [tip:x86/asm] x86/asm/entry/64: Migrate error and IRQ exit work to C and remove old assembly code tip-bot for Andy Lutomirski
2015-08-11 22:18     ` Frederic Weisbecker
2015-08-11 22:25       ` Andy Lutomirski
2015-08-11 22:49         ` Frederic Weisbecker
2015-08-11 22:59           ` Andy Lutomirski
2015-08-12  1:02             ` Paul E. McKenney
2015-08-12 13:13             ` Frederic Weisbecker
2015-08-11 22:38     ` Frederic Weisbecker
2015-08-11 22:51       ` Andy Lutomirski
2015-08-11 23:22         ` Frederic Weisbecker
2015-08-11 23:33           ` Andy Lutomirski
2015-08-12 13:32             ` Frederic Weisbecker
2015-08-12 14:59               ` Andy Lutomirski
2015-08-18 22:34                 ` Frederic Weisbecker
2015-08-18 22:40                   ` Andy Lutomirski
2015-08-19 17:18                     ` Frederic Weisbecker
2015-08-19 18:02                       ` Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 15/17] x86/entry: Remove exception_enter from most trap handlers Andy Lutomirski
2015-07-07 10:53   ` [tip:x86/asm] x86/entry: Remove exception_enter() " tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 16/17] x86/entry: Remove SCHEDULE_USER and asm/context-tracking.h Andy Lutomirski
2015-07-07 10:54   ` [tip:x86/asm] x86/entry: Remove SCHEDULE_USER and asm/ context-tracking.h tip-bot for Andy Lutomirski
2015-07-03 19:44 ` [PATCH v5 17/17] x86/irq: Document how IRQ context tracking works and add an assertion Andy Lutomirski
2015-07-07 10:54   ` [tip:x86/asm] x86/irq, context_tracking: Document how IRQ context tracking works and add an RCU assertion tip-bot for Andy Lutomirski
2015-07-14 23:26     ` Frederic Weisbecker
2015-07-14 23:33       ` Andy Lutomirski
2015-07-18 13:23         ` Frederic Weisbecker
2015-07-18 14:10           ` Paul E. McKenney [this message]
2015-07-07 11:12 ` [PATCH v5 00/17] x86: Rewrite exit-to-userspace code Ingo Molnar
2015-07-07 16:03   ` Andy Lutomirski
2015-07-07 17:55     ` [PATCH] x86/entry/64: Fix warning on compat syscalls with CONFIG_AUDITSYSCALL=n Andy Lutomirski
2015-07-08  9:57       ` [tip:x86/asm] x86/entry/64: Fix IRQ state confusion and related warning on compat syscalls with CONFIG_AUDITSYSCALL =n tip-bot for Andy Lutomirski
2015-07-08 19:12       ` tip-bot for Andy Lutomirski

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=20150718141013.GS3717@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=bp@alien8.de \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.com \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git