lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <941567906.21756.1444207325021.JavaMail.zimbra@efficios.com> Date: Wed, 7 Oct 2015 08:42:05 +0000 (UTC) From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com> To: Peter Zijlstra <peterz@...radead.org> Cc: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>, linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>, Lai Jiangshan <jiangshanlai@...il.com>, dipankar@...ibm.com, Andrew Morton <akpm@...ux-foundation.org>, josh@...htriplett.org, Thomas Gleixner <tglx@...utronix.de>, rostedt <rostedt@...dmis.org>, dhowells@...hat.com, edumazet@...gle.com, dvhart@...ux.intel.com, fweisbec@...il.com, oleg@...hat.com, bobby prani <bobby.prani@...il.com> Subject: Re: [PATCH tip/core/rcu 02/18] rcu: Move rcu_report_exp_rnp() to allow consolidation ----- On Oct 7, 2015, at 3:51 AM, Peter Zijlstra peterz@...radead.org wrote: > On Tue, Oct 06, 2015 at 01:58:50PM -0700, Paul E. McKenney wrote: >> On Tue, Oct 06, 2015 at 10:29:37PM +0200, Peter Zijlstra wrote: >> > On Tue, Oct 06, 2015 at 09:29:21AM -0700, Paul E. McKenney wrote: >> > > +static void __maybe_unused rcu_report_exp_rnp(struct rcu_state *rsp, >> > > + struct rcu_node *rnp, bool wake) >> > > +{ >> > > + unsigned long flags; >> > > + unsigned long mask; >> > > + >> > > + raw_spin_lock_irqsave(&rnp->lock, flags); >> > >> > Normally we require a comment with barriers, explaining the order and >> > the pairing etc.. :-) >> > >> > > + smp_mb__after_unlock_lock(); >> >> Hmmmm... That is not good. >> >> Worse yet, I am missing comments on most of the pre-existing barriers >> of this form. > > Yes I noticed.. :/ > >> The purpose is to enforce the heavy-weight grace-period memory-ordering >> guarantees documented in the synchronize_sched() header comment and >> elsewhere. > >> They pair with anything you might use to check for violation >> of these guarantees, or, simiarly, any ordering that you might use when >> relying on these guarantees. > > I'm sure you know what that means, but I've no clue ;-) That is, I > wouldn't know where to start looking in the RCU implementation to verify > the barrier is either needed or sufficient. Unless you mean _everywhere_ > :-) One example is the new membarrier system call. It relies on synchronize_sched() to enforce this: from kernel/membarrier.c: * All memory accesses performed in program order from each targeted thread * is guaranteed to be ordered with respect to sys_membarrier(). If we use * the semantic "barrier()" to represent a compiler barrier forcing memory * accesses to be performed in program order across the barrier, and * smp_mb() to represent explicit memory barriers forcing full memory * ordering across the barrier, we have the following ordering table for * each pair of barrier(), sys_membarrier() and smp_mb(): * * The pair ordering is detailed as (O: ordered, X: not ordered): * * barrier() smp_mb() sys_membarrier() * barrier() X X O * smp_mb() X O O * sys_membarrier() O O O And include/uapi/linux/membarrier.h: * @MEMBARRIER_CMD_SHARED: Execute a memory barrier on all running threads. * Upon return from system call, the caller thread * is ensured that all running threads have passed * through a state where all memory accesses to * user-space addresses match program order between * entry to and return from the system call * (non-running threads are de facto in such a * state). This covers threads from all processes * running on the system. This command returns 0. I hope this sheds light on a userspace-facing interface to synchronize_sched() and clarifies its expected semantic a bit. Thanks, Mathieu > >> I could add something like "/* Enforce GP memory ordering. */" >> >> Or perhaps "/* See synchronize_sched() header. */" >> >> I do not propose reproducing the synchronize_sched() header on each >> of these. That would be verbose, even for me! ;-) >> >> Other thoughts? > > Well, this is an UNLOCK+LOCK on non-matching lock variables upgrade to > full barrier thing, right? > > To me its not clear which UNLOCK we even match here. I've just read the > sync_sched() header, but that doesn't help me either, so referring to > that isn't really helpful either. > > In any case, I don't want to make too big a fuzz here, but I just > stumbled over a lot of unannotated barriers and figured I ought to say > something about it. -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists