Re: [PATCH] RCU: implement rcu_read_[un]lock_preempt()

[Peter Zijlstra <peterz@infradead.org>]

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@kernel.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.