All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolas Saenz Julienne <nsaenzju@redhat.com>
To: paulmck@kernel.org, Thomas Gleixner <tglx@linutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	rcu@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>,
	mtosatti <mtosatti@redhat.com>, frederic <frederic@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>
Subject: Re: [PATCH v2] Documentation: Fill the gaps about entry/noinstr constraints
Date: Mon, 13 Dec 2021 11:36:56 +0100	[thread overview]
Message-ID: <09e4a37b390479baa4e0947670d49c44d58c2855.camel@redhat.com> (raw)
In-Reply-To: <20211203200808.GT641268@paulmck-ThinkPad-P17-Gen-1>

Hi All,
now that this is good shape I'm taking over Thomas and preparing v3.

Paul, I introduced most (if not all) your paragraph corrections. Some questions
below.

On Fri, 2021-12-03 at 12:08 -0800, Paul E. McKenney wrote:
> > +The update order depends on the transition type and is explained below in
> > +the transition type sections.
> @@@

Sorry, I'm not 100% sure I get what you meant by this. Maybe introducing some
sort of link?

[...]

> > +syscall_exit_to_user_mode() handles all work which needs to be done before
> > +returning to user space like tracing, audit, signals, task work etc. After
> > +that it invokes exit_to_user_mode() which again handles the state
> > +transition in the reverse order:
> > +
> > +  * Tracing
> > +  * RCU / Context tracking
> > +  * Lockdep
> > +
> > +syscall_enter_from_user_mode() and syscall_exit_to_user_mode() are also
> > +available as fine grained subfunctions in cases where the architecture code
> > +has to do extra work between the various steps. In such cases it has to
> > +ensure that enter_from_user_mode() is called first on entry and
> > +exit_to_user_mode() is called last on exit.
> 
> !!! Here I have a question.  Can calls to enter_from_user_mode()
> be nested?  RCU is OK with this, but I am not so sure that everything
> else is.  If nesting is prohibited, this paragraph should explicitly
> say that.  If nesting is theoretically possible, but should be avoided,
> it would be good to say that as well.  (Otherwise "It looks like it
> might work, so let's go for it!")


In __enter_from_user_mode() I see:

	CT_WARN_ON(ct_state() != CONTEXT_USER);

IIUC this signals that a nested syscall entry isn't expected from CT's point of
view. I remember reading through RCU's dyntick code that the rationale for
nesting in the syscall path was half interrupts (or upcalls). I did some
research, but couldn't find an example of this. Is this something we can
discard as an old technique not used anymore?

On the other hand, interrupts are prone to nesting:
 - Weird interrupt handlers that re-enable interrupts
 - NMIs interrupting Hard IRQ context
 - NMIs interrupting NMIs

Please let me know if I'm off-base, but I think the topic of nesting is worth a
sentence or two in each section.

[...]

> > +Interrupts and regular exceptions
> > +---------------------------------
> > +
> > +Interrupts entry and exit handling is slightly more complex than syscalls
> > +and KVM transitions.
> > +
> > +If an interrupt is raised while the CPU executes in user space, the entry
> > +and exit handling is exactly the same as for syscalls.
> > +
> > +If the interrupt is raised while the CPU executes in kernel space the entry
> > +and exit handling is slightly different. RCU state is only updated when the
> > +interrupt was raised in context of the CPU's idle task because that's the
> > +only kernel context where RCU can be not watching on NOHZ enabled kernels.
> > +Lockdep and tracing have to be updated unconditionally.
> 
> !!! You lost me on this one.  Does that second-to-last sentence instead
> want to end something like this?  "... where RCU will not be watching
> when running on non-nohz_full CPUs."

The paragraph covers IRQ entry from kernel space. In that context RCU is only
shut-off during idle. That only happens on a NOHZ-enabled kernel, be it
NO_HZ_IDLE or NO_HZ_FULL.

I'll try to reword it a bit so it's more explicit.

Thanks!

-- 
Nicolás Sáenz


WARNING: multiple messages have this Message-ID (diff)
From: Nicolas Saenz Julienne <nsaenzju@redhat.com>
To: paulmck@kernel.org, Thomas Gleixner <tglx@linutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	 linux-kernel <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	 rcu@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>,
	mtosatti <mtosatti@redhat.com>, frederic <frederic@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>
Subject: Re: [PATCH v2] Documentation: Fill the gaps about entry/noinstr constraints
Date: Mon, 13 Dec 2021 11:36:56 +0100	[thread overview]
Message-ID: <09e4a37b390479baa4e0947670d49c44d58c2855.camel@redhat.com> (raw)
In-Reply-To: <20211203200808.GT641268@paulmck-ThinkPad-P17-Gen-1>

Hi All,
now that this is good shape I'm taking over Thomas and preparing v3.

Paul, I introduced most (if not all) your paragraph corrections. Some questions
below.

On Fri, 2021-12-03 at 12:08 -0800, Paul E. McKenney wrote:
> > +The update order depends on the transition type and is explained below in
> > +the transition type sections.
> @@@

Sorry, I'm not 100% sure I get what you meant by this. Maybe introducing some
sort of link?

[...]

> > +syscall_exit_to_user_mode() handles all work which needs to be done before
> > +returning to user space like tracing, audit, signals, task work etc. After
> > +that it invokes exit_to_user_mode() which again handles the state
> > +transition in the reverse order:
> > +
> > +  * Tracing
> > +  * RCU / Context tracking
> > +  * Lockdep
> > +
> > +syscall_enter_from_user_mode() and syscall_exit_to_user_mode() are also
> > +available as fine grained subfunctions in cases where the architecture code
> > +has to do extra work between the various steps. In such cases it has to
> > +ensure that enter_from_user_mode() is called first on entry and
> > +exit_to_user_mode() is called last on exit.
> 
> !!! Here I have a question.  Can calls to enter_from_user_mode()
> be nested?  RCU is OK with this, but I am not so sure that everything
> else is.  If nesting is prohibited, this paragraph should explicitly
> say that.  If nesting is theoretically possible, but should be avoided,
> it would be good to say that as well.  (Otherwise "It looks like it
> might work, so let's go for it!")


In __enter_from_user_mode() I see:

	CT_WARN_ON(ct_state() != CONTEXT_USER);

IIUC this signals that a nested syscall entry isn't expected from CT's point of
view. I remember reading through RCU's dyntick code that the rationale for
nesting in the syscall path was half interrupts (or upcalls). I did some
research, but couldn't find an example of this. Is this something we can
discard as an old technique not used anymore?

On the other hand, interrupts are prone to nesting:
 - Weird interrupt handlers that re-enable interrupts
 - NMIs interrupting Hard IRQ context
 - NMIs interrupting NMIs

Please let me know if I'm off-base, but I think the topic of nesting is worth a
sentence or two in each section.

[...]

> > +Interrupts and regular exceptions
> > +---------------------------------
> > +
> > +Interrupts entry and exit handling is slightly more complex than syscalls
> > +and KVM transitions.
> > +
> > +If an interrupt is raised while the CPU executes in user space, the entry
> > +and exit handling is exactly the same as for syscalls.
> > +
> > +If the interrupt is raised while the CPU executes in kernel space the entry
> > +and exit handling is slightly different. RCU state is only updated when the
> > +interrupt was raised in context of the CPU's idle task because that's the
> > +only kernel context where RCU can be not watching on NOHZ enabled kernels.
> > +Lockdep and tracing have to be updated unconditionally.
> 
> !!! You lost me on this one.  Does that second-to-last sentence instead
> want to end something like this?  "... where RCU will not be watching
> when running on non-nohz_full CPUs."

The paragraph covers IRQ entry from kernel space. In that context RCU is only
shut-off during idle. That only happens on a NOHZ-enabled kernel, be it
NO_HZ_IDLE or NO_HZ_FULL.

I'll try to reword it a bit so it's more explicit.

Thanks!

-- 
Nicolás Sáenz


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-12-13 10:38 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-30 11:28 Question WRT early IRQ/NMI entry code Nicolas Saenz Julienne
2021-11-30 11:28 ` Nicolas Saenz Julienne
2021-11-30 12:05 ` Frederic Weisbecker
2021-11-30 12:05   ` Frederic Weisbecker
2021-11-30 12:50 ` Mark Rutland
2021-11-30 12:50   ` Mark Rutland
2021-11-30 13:47 ` Thomas Gleixner
2021-11-30 13:47   ` Thomas Gleixner
2021-11-30 14:13   ` Steven Rostedt
2021-11-30 14:13     ` Steven Rostedt
2021-11-30 22:31     ` [PATCH] Documentation: Fill the gaps about entry/noinstr constraints Thomas Gleixner
2021-11-30 22:31       ` Thomas Gleixner
2021-12-01 10:56       ` Mark Rutland
2021-12-01 10:56         ` Mark Rutland
2021-12-01 18:14         ` Thomas Gleixner
2021-12-01 18:14           ` Thomas Gleixner
2021-12-01 18:23           ` Mark Rutland
2021-12-01 18:23             ` Mark Rutland
2021-12-01 20:28             ` Thomas Gleixner
2021-12-01 20:28               ` Thomas Gleixner
2021-12-01 20:35               ` [PATCH v2] " Thomas Gleixner
2021-12-01 20:35                 ` Thomas Gleixner
2021-12-02 10:03                 ` Mark Rutland
2021-12-02 10:03                   ` Mark Rutland
2021-12-03 20:08                 ` Paul E. McKenney
2021-12-03 20:08                   ` Paul E. McKenney
2021-12-13 10:36                   ` Nicolas Saenz Julienne [this message]
2021-12-13 10:36                     ` Nicolas Saenz Julienne
2021-12-13 16:41                     ` Paul E. McKenney
2021-12-13 16:41                       ` Paul E. McKenney
2021-12-04  3:48                 ` Randy Dunlap
2021-12-04  3:48                   ` Randy Dunlap
2021-12-06 17:36                   ` Mark Rutland
2021-12-06 17:36                     ` Mark Rutland
2021-12-06 17:53                     ` Paul E. McKenney
2021-12-06 17:53                       ` Paul E. McKenney
2021-12-06 21:24                       ` Randy Dunlap
2021-12-06 21:24                         ` Randy Dunlap
2021-12-06 21:36                         ` Paul E. McKenney
2021-12-06 21:36                           ` Paul E. McKenney
2021-11-30 15:13   ` Question WRT early IRQ/NMI entry code Nicolas Saenz Julienne
2021-11-30 15:13     ` Nicolas Saenz Julienne

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=09e4a37b390479baa4e0947670d49c44d58c2855.camel@redhat.com \
    --to=nsaenzju@redhat.com \
    --cc=corbet@lwn.net \
    --cc=frederic@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mtosatti@redhat.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /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.