+ 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 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? There are a few outstanding points that we're trying to talk through, but it should be fine for an initial implementation to start with the amoadd-based approach. As far as the ARM LSE atomic implementation goes, I'm not an expert on those instructions. If those instructions are locked across all of the caches for the cores in the Linux system also, then they probably don't need the preempt_disable/enable either - assuming our collective understanding of the purpose of the preempt_disable/enable is correct. All this is, of course, assuming there is no secondary purpose to the preempt_disable/enable that we haven't managed to elicit yet. - Paul