Re: per-cpu thoughts

From: Nick Kossifidis <mick@ics.forth.gr>
To: Paul Walmsley <paul.walmsley@sifive.com>
Cc: "Paul Walmsley" <paul@pwsan.com>,
	"Björn Töpel" <bjorn.topel@gmail.com>,
	"Palmer Dabbelt" <palmer@sifive.com>,
	will.deacon@arm.com, catalin.marinas@arm.com,
	"Nick Kossifidis" <mick@ics.forth.gr>,
	"Christopher Lameter" <cl@linux.com>,
	linux-riscv@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: per-cpu thoughts
Date: Fri, 22 Mar 2019 16:51:33 +0200	[thread overview]
Message-ID: <06e2cd9f9dbaf01fec2f910ebb402c2d@mailhost.ics.forth.gr> (raw)
In-Reply-To: <alpine.DEB.2.21.9999.1903110814130.11892@viisi.sifive.com>

Στις 2019-03-11 17:26, Paul Walmsley έγραψε:
> + the ARM64 guys and lakml
> 
> On Mon, 11 Mar 2019, Björn Töpel wrote:
> 
>> On Mon, 11 Mar 2019 at 15:56, Christopher Lameter <cl@linux.com> 
>> wrote:
>> >
>> > On Mon, 11 Mar 2019, Björn Töpel wrote:
>> >
>> > > > Thanks a bunch!  I feel like the best option here is to just use the AMOs
>> > > > without disabling preemption and ensuring that all other accesses are atomic
>> > > > (via AMOs or LR/SC).  The only reason I can see that wouldn't be the way to go
>> > > > would be if it requires non-arch modifications, as if we go down that path
>> > > > we'll be able to handle the performance edge cases in the implementation.
>> > > >
>> > >
>> > > Hmm, you mean AMO *with* preempt_{dis,en}able, right? (No, interrupt
>> > > disable, only preemption.)
>> >
>> > If you disable preemption then you dont need AMO anymore. In fact that is
>> > the default fallback generic implementation. Just dont do anything and you
>> > already have that solution.
>> 
>> But the generic one disables interrupts, right?
>> 
>> I believe the rational for RV is similar to ARM's; AMO+preemption
>> disable regions is *slightly* better than the generic, but not as good
>> as the IA one. Or am I missing something?
> 
> There's been a discussion going on in a private thread about this that 
> I
> unfortunately didn't add you to.  The discussion is still ongoing, but 
> I
> think Christoph and myself and a few other folks have agreed that the
> preempt_disable/enable is not needed for the amoadd approach.  This is
> since the apparent intention of the preemption disable/enable is to 
> ensure
> the correctness of the counter increment; however there is no risk of
> incorrectness in an amoadd sequence since the atomic add is locked 
> across
> all of the cache coherency domain.  Christoph, would you disagree with
> that characterization?
> 

Some further input on this...

https://www.kernel.org/doc/Documentation/preempt-locking.txt

"Note that you do not need to explicitly prevent preemption if you are 
holding
any locks or interrupts are disabled, since preemption is implicitly 
disabled
in those cases.

But keep in mind that 'irqs disabled' is a fundamentally unsafe way of
disabling preemption - any cond_resched() or cond_resched_lock() might 
trigger
a reschedule if the preempt count is 0. A simple printk() might trigger 
a
reschedule. So use this implicit preemption-disabling property only if 
you
know that the affected codepath does not do any of this. Best policy is 
to use
this only for small, atomic code that you wrote and which calls no 
complex
functions."

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv