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
| ||
|
Date: Mon, 10 Feb 2020 15:30:08 -0500 From: Joel Fernandes <joel@...lfernandes.org> To: Steven Rostedt <rostedt@...dmis.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Peter Zijlstra <peterz@...radead.org>, linux-kernel <linux-kernel@...r.kernel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Gustavo A. R. Silva" <gustavo@...eddedor.com>, Ingo Molnar <mingo@...hat.com>, Richard Fontana <rfontana@...hat.com>, Thomas Gleixner <tglx@...utronix.de>, paulmck <paulmck@...nel.org>, Josh Triplett <josh@...htriplett.org>, Lai Jiangshan <jiangshanlai@...il.com>, Arnaldo Carvalho de Melo <arnaldo.melo@...il.com> Subject: Re: [RFC 0/3] Revert SRCU from tracepoint infrastructure Hi Steve, On Mon, Feb 10, 2020 at 03:03:48PM -0500, Steven Rostedt wrote: > On Mon, 10 Feb 2020 14:53:02 -0500 > Joel Fernandes <joel@...lfernandes.org> wrote: > > > > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h > > > index 1fb11daa5c53..a83fd076a312 100644 > > > --- a/include/linux/tracepoint.h > > > +++ b/include/linux/tracepoint.h > > > @@ -179,10 +179,8 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p) > > > * For rcuidle callers, use srcu since sched-rcu \ > > > * doesn't work from the idle path. \ > > > */ \ > > > - if (rcuidle) { \ > > > + if (rcuidle) \ > > > __idx = srcu_read_lock_notrace(&tracepoint_srcu);\ > > > - rcu_irq_enter_irqson(); \ > > > - } \ > > > > This would still break out-of-tree modules or future code that does > > rcu_read_lock() right in a tracepoint callback right? > > Yes, and that's fine. > > > > > Or are we saying that rcu_read_lock() in a tracepoint callback is not > > allowed? I believe this should then at least be documented somewhere. Also, > > No, it's only not allowed if you you attached to a tracepoint that can > be called without rcu watching. That's up to the caller to figure it > out. Tracepoints were never meant to be a generic thing people should > use without knowing what they are really doing. Ok, right. > > what about code in tracepoint callback that calls rcu_read_lock() indirectly > > through a path in the kernel, and also code that may expect RCU readers when > > doing preempt_disable()? > > Then they need to know what they are doing. Ok. > > So basically we are saying with this patch: > > 1. Don't call in a callback: rcu_read_lock() or preempt_disable() and expect RCU to do > > anything for you. > > We can just say, "If you plan on using RCU, be aware that it man not be > watching and you get do deal with the fallout. Use rcu_is_watching() to > figure it out." Ok. > > 2. Don't call code that does anything that 1. needs. > > > > Is that intended? thanks, > > > > No, look what the patch did for perf. Why make *all* callbacks suffer > if only some use RCU? If you use RCU from a callback, then you need to > figure it out. The same goes for attaching to the function tracer. Only the callbacks on the rcuidle ones would suffer though, not all callbacks. Yes I saw the patch, it looks like a good idea to me and I am Ok with it. thanks, - Joel
Powered by blists - more mailing lists