linux-kernel - Re: [PATCH] RCU: implement rcu_read_[un]lock

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: <1217258001.18049.7.camel@twins>
Date:	Mon, 28 Jul 2008 17:13:21 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Tejun Heo <tj@...nel.org>
Cc:	dipankar@...ibm.com, Andrew Morton <akpm@...ux-foundation.org>,
	Jens Axboe <jens.axboe@...cle.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Paul E McKenney <paulmck@...ux.vnet.ibm.com>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] RCU: implement rcu_read_[un]lock_preempt()

On Wed, 2008-07-16 at 15:43 +0900, Tejun Heo wrote:
> Peter Zijlstra wrote:
> > On Mon, 2008-07-14 at 14:57 +0900, Tejun Heo wrote:
> >> With the introduction of preemptible RCU, RCU doesn't gurantee that
> >> its critical section runs on the CPU it started to run.  As there are
> >> cases where non-preemptible RCU critical section makes sense, create
> >> new RCU read lock variants which turns of preemption -
> >> rcu_read_[un]lock_preempt() which are identical to rcu_read_[un]lock()
> >> for classic implementation and have enclosing preempt disable/enable
> >> for preemptible RCU.
> >>
> >> Signed-off-by: Tejun Heo <tj@...nel.org>
> > 
> > Sorry, NAK.
> > 
> > If you need preempt off you need it for other reasons than RCU, so
> > mixing it in the interface doesn't make sense to me.
> 
> Hmmm... the point of the interface is avoiding doing double preemption
> operations as on common configurations rcu_read_lock() disables
> preemption.

Should be really cheap then, because the cacheline is already hot.

>   Yes, it's for different purposes but we have two partially
> overlapping ops and implementing combined / collapsed ops for such cases
> is acceptable, I think.

They only overlap for !PREEMPT_RCU || !PREEMPT_RT

> Using get_cpu() or separate preempt_disable() wouldn't incur noticeable
> performance difference as preemption is really cheap to manipulate but
> both per-cpu and RCU are for performance optimization and I think having
> combined ops is a good idea.

I don't as its a nightmare to sort out on -rt, where get_cpu() can be
converted to get_cpu_locked(), and rcu_read_lock() never disables
preemption.

If you convert it to use get_cpu() the conversion is easy, if you
introduce this collapsed primitive we're up shit creek because it
doesn't map.

Nor does it tell us why you need preempt disabled. Making it just as bad
as open-coded preempt_disable()s.

--
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/