linux-kernel - Re: How do I increment a per-CPU variable without warning?

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140416051909.GJ26782@laptop.programming.kicks-ass.net>
Date:	Wed, 16 Apr 2014 07:19:09 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	cl@...ux.com, linux-kernel@...r.kernel.org, mingo@...nel.org,
	tj@...nel.org, grygorii.strashko@...com
Subject: Re: How do I increment a per-CPU variable without warning?

On Tue, Apr 15, 2014 at 03:17:55PM -0700, Paul E. McKenney wrote:
> Hello, Christoph,
> 
> I have a patch that currently uses __this_cpu_inc_return() to increment a
> per-CPU variable, but without preemption disabled.  Of course, given that
> preemption is enabled, it might well end up picking up one CPU's counter,
> adding one to it, then storing the result into some other CPU's counter.
> But this is OK, the test can be probabilistic.  And when I run this
> against v3.14 and earlier, it works fine.
> 
> But now there is 188a81409ff7 (percpu: add preemption checks to
> __this_cpu ops), which gives me lots of splats.
> 
> My current admittedly crude workaround is as follows:
> 
> 	static inline bool rcu_should_resched(void)
> 	{
> 		int t;
> 
> 	#ifdef CONFIG_DEBUG_PREEMPT
> 		preempt_disable();
> 	#endif /* #ifdef CONFIG_DEBUG_PREEMPT */
> 		t = __this_cpu_read(rcu_cond_resched_count) + 1;
> 		if (t < RCU_COND_RESCHED_LIM) {
> 			__this_cpu_write(rcu_cond_resched_count, t);
> 	#ifdef CONFIG_DEBUG_PREEMPT
> 			preempt_enable();
> 	#endif /* #ifdef CONFIG_DEBUG_PREEMPT */
> 			return false;
> 		}
> 	#ifdef CONFIG_DEBUG_PREEMPT
> 		preempt_enable();
> 	#endif /* #ifdef CONFIG_DEBUG_PREEMPT */
> 		return true;
> 	}
> 
> This is arguably better than the original __this_cpu_read() because it
> avoids overflow, but I thought I should check to see if there was some
> better way to do this.

you could use raw_cpu_{read,write}(). But note that without the
unconditional preempt_disable() in there your code can read a different
rcu_cond_resched_count than it writes.

So I think you very much want that preempt_disable().
--
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/