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: Fri, 20 Nov 2020 14:27:18 +0100 From: Thomas Gleixner <tglx@...utronix.de> To: Frederic Weisbecker <frederic@...nel.org> Cc: LKML <linux-kernel@...r.kernel.org>, Peter Zijlstra <peterz@...radead.org>, Paul McKenney <paulmck@...nel.org>, Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Arnd Bergmann <arnd@...db.de>, "James E.J. Bottomley" <James.Bottomley@...senPartnership.com>, Helge Deller <deller@....de>, linux-parisc@...r.kernel.org, Yoshinori Sato <ysato@...rs.sourceforge.jp>, Rich Felker <dalias@...c.org>, linux-sh@...r.kernel.org, Jeff Dike <jdike@...toit.com>, Richard Weinberger <richard@....at>, Anton Ivanov <anton.ivanov@...bridgegreys.com>, linux-um@...ts.infradead.org, Russell King <linux@...linux.org.uk>, Marc Zyngier <maz@...nel.org>, Valentin Schneider <valentin.schneider@....com>, linux-arm-kernel@...ts.infradead.org, Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org> Subject: Re: [patch 14/19] softirq: Make softirq control and processing RT aware On Fri, Nov 20 2020 at 01:26, Frederic Weisbecker wrote: > On Fri, Nov 13, 2020 at 03:02:21PM +0100, Thomas Gleixner wrote: >> +void __local_bh_disable_ip(unsigned long ip, unsigned int cnt) >> +{ >> + unsigned long flags; >> + int newcnt; >> + >> + WARN_ON_ONCE(in_hardirq()); >> + >> + /* First entry of a task into a BH disabled section? */ >> + if (!current->softirq_disable_cnt) { >> + if (preemptible()) { >> + local_lock(&softirq_ctrl.lock); >> + rcu_read_lock(); > > Ah you lock RCU because local_bh_disable() implies it and > since it doesn't disable preemption anymore, you must do it > explicitly? > > Perhaps local_lock() should itself imply rcu_read_lock() ? It's really only required for local_bh_disable(). Lemme add a comment. >> + } else { >> + DEBUG_LOCKS_WARN_ON(this_cpu_read(softirq_ctrl.cnt)); >> + } >> + } >> + >> + preempt_disable(); > > Do you really need to disable preemption here? Migration is disabled by local_lock() > and I can't figure out a scenario where the below can conflict with a > preempting task. Indeed it's pointless. >> + /* >> + * Track the per CPU softirq disabled state. On RT this is per CPU >> + * state to allow preemption of bottom half disabled sections. >> + */ >> + newcnt = this_cpu_add_return(softirq_ctrl.cnt, cnt); > > __this_cpu_add_return() ? Yep. Thanks, tglx
Powered by blists - more mailing lists