Hi Mark,

On Tue, 12 Mar 2019, Mark Rutland wrote:

On Mon, Mar 11, 2019 at 11:39:56AM -0700, Paul Walmsley wrote:

quoted

My understanding is that many of Christoph's per-cpu performance concerns 
revolve around counters in the VM code, such as:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/vmstat.c#n355

The mod_*_state() functions are the only ones which mess with
preemption, and that should only mandate a few locally-visible
modifications of preempt_count.

Also __{inc,dec}_*_state() calls __this_cpu_{inc,dec}_return() which 
tweaks the preemption count.

Similar cases apply within SLUB, and I'd hoped to improve that with my
this-cpu-reg branch, but I didn't see a measureable improvement on
workloads I tried.

That certainly suggests that all of this could be much to-do about 
nothing, or at least very little.  One observation is that some of the 
performance concerns that Christoph is expressing here may be about 
ensuring predictable and minimal latency bounds, rather than raw 
throughput.

Have you seen a measureable performance problem here?

Not yet.  The two motivations at the moment are:

1. to determine how our initial per-arch implementation for percpu.h 
should look, and 

2. to get a high-level view of whether unlocked base + offset increment 
instructions are worthwhile, from people who know more than I do about 
them.

So far the counters look like a distinct use-case - one that might have 
relaxed requirements wrt preemption changes.

quoted

and probably elsewhere by now.  It may be worth creating a distinct API 
for those counters.  If only increment, decrement, and read operations are 
needed, there shouldn't be a need to disable or re-enable 
preemption in those code paths - assuming that one is either able to 
tolerate the occasional cache line bounce or retries in a long LL/SC 
sequence.  Any opinions on that?

I'm afraid I don't understand this code well enough to say whether that
would be safe.

That makes two of us.  Have followed up with Christoph in a separate 
thread with lakml cc'ed.

It's not clear to me whether there would be a measureable performance
difference, as I'd expect fiddling with preempt_count to be relatively
cheap. The AMOs themselves don't need to enforce ordering here, and only
a few compiler barriers are necessary.

OK.  I have been assuming that the risk of a scheduler call in 
preempt_enable() is what Christoph is concerned about here:

https://lore.kernel.org/linux-riscv/b0653f7a6f1bc0c9329d37de690d3bed@mailhost.ics.forth.gr/T/#m6e609e26a9e5405c4a7e2dbd5ca8c969cada5c36 (local)

If is possible to eliminate the latency risk from a 'simple' counter 
increment/decrement by creating a restricted API, that may be worthwhile.

Christoph has also been concerned that the AMO operations will carry an 
unacceptable performance overhead.  But the RISC-V AMO operations can be 
written such that they don't have the ordering restrictions that the Intel 
LOCK-prefixed operations do, and thus those concerns may not apply -- at 
least not to the same extent.  Perhaps this is also true for the ARM LSE 
atomics.


- Paul

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