* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Peter Zijlstra @ 2019-03-14 19:18 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190314150753.lzu6uftirxnhdv4y@e110439-lin>
On Thu, Mar 14, 2019 at 03:07:53PM +0000, Patrick Bellasi wrote:
> On 14-Mar 14:32, Peter Zijlstra wrote:
> > On Thu, Mar 14, 2019 at 12:13:15PM +0000, Patrick Bellasi wrote:
> > > > I'd be most impressed if they pull this off. Check the generated code
> > > > and see I suppose :-)
> > >
> > > On x86 the code generated looks exactly the same:
> > >
> > > https://godbolt.org/z/PjmA7k
> >
> > Argh, they do mult by inverse to avoid the division, and can do this
> > because its a constant.
> >
> > And then yes, your arm version looks worse.
>
> your "arm version" is worst then x86, or "your version" is worse?
>
> IOW, should I keep the code as the original? Do you prefer your
> version? Or... we don't really care...
Yeah, keep the original, it didn't matter on x86 and arm regressed with
my version.
> > It does what I expected with -Os, but as Rutland said the other day,
> > that stands for Optimize for Sadness.
>
> Yes, I guess we cannot optimize for all flags... however, just let me
> know what you prefer and I'll put that version in ;)
Yeah, don't bother optimizing for -Os, it generally creates atrocious
crap, hence Rutland calling it 'Optimize for Sadness'.
^ permalink raw reply
* [RFC PATCH 2/2] rseq/selftests: Introduce __rseq_cs_ptr_array, rename __rseq_table to __rseq_cs
From: Mathieu Desnoyers @ 2019-03-14 19:03 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk
In-Reply-To: <20190314190346.24744-1-mathieu.desnoyers@efficios.com>
The entries within __rseq_table are aligned on 32 bytes due to
linux/rseq.h struct rseq_cs uapi requirements, but the start of the
__rseq_table section is not guaranteed to be 32-byte aligned. It can
cause padding to be added at the start of the section, which makes it
hard to use as an array of items by debuggers.
Considering that __rseq_table does not really consist of a table due to
the presence of padding, rename this section to __rseq_cs.
Create a new __rseq_cs_ptr_array section which contains 64-bit packed
pointers to entries within the __rseq_cs section.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Joel Fernandes <joelaf@google.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Dave Watson <davejwatson@fb.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Shuah Khan <shuah@kernel.org>
CC: Andi Kleen <andi@firstfloor.org>
CC: linux-kselftest@vger.kernel.org
CC: "H . Peter Anvin" <hpa@zytor.com>
CC: Chris Lameter <cl@linux.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>
CC: Paul Turner <pjt@google.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ben Maurer <bmaurer@fb.com>
CC: linux-api@vger.kernel.org
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
---
tools/testing/selftests/rseq/rseq-arm.h | 32 +++++++++++++++++--------------
tools/testing/selftests/rseq/rseq-arm64.h | 9 ++++++---
tools/testing/selftests/rseq/rseq-mips.h | 32 +++++++++++++++++--------------
tools/testing/selftests/rseq/rseq-ppc.h | 22 +++++++++++++--------
tools/testing/selftests/rseq/rseq-s390.h | 18 +++++++++++------
tools/testing/selftests/rseq/rseq-x86.h | 19 ++++++++++++------
6 files changed, 81 insertions(+), 51 deletions(-)
diff --git a/tools/testing/selftests/rseq/rseq-arm.h b/tools/testing/selftests/rseq/rseq-arm.h
index 17e8d231943a..5f262c54364f 100644
--- a/tools/testing/selftests/rseq/rseq-arm.h
+++ b/tools/testing/selftests/rseq/rseq-arm.h
@@ -30,24 +30,28 @@ do { \
#include "rseq-skip.h"
#else /* !RSEQ_SKIP_FASTPATH */
-#define __RSEQ_ASM_DEFINE_TABLE(version, flags, start_ip, \
+#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, start_ip, \
post_commit_offset, abort_ip) \
- ".pushsection __rseq_table, \"aw\"\n\t" \
+ ".pushsection __rseq_cs, \"aw\"\n\t" \
".balign 32\n\t" \
+ __rseq_str(label) ":\n\t" \
".word " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
".word " __rseq_str(start_ip) ", 0x0, " __rseq_str(post_commit_offset) ", 0x0, " __rseq_str(abort_ip) ", 0x0\n\t" \
+ ".popsection\n\t" \
+ ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
+ ".word " __rseq_str(label) "b, 0x0\n\t" \
".popsection\n\t"
-#define RSEQ_ASM_DEFINE_TABLE(start_ip, post_commit_ip, abort_ip) \
- __RSEQ_ASM_DEFINE_TABLE(0x0, 0x0, start_ip, \
+#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
+ __RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
(post_commit_ip - start_ip), abort_ip)
/*
* Exit points of a rseq critical section consist of all instructions outside
* of the critical section where a critical section can either branch to or
* reach through the normal course of its execution. The abort IP and the
- * post-commit IP are already part of the __rseq_table section and should not
- * be explicitly defined as additional exit points. Knowing all exit points is
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
* useful to assist debuggers stepping over the critical section.
*/
#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
@@ -99,7 +103,7 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -166,7 +170,7 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -237,7 +241,7 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
#endif
@@ -292,7 +296,7 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -367,7 +371,7 @@ int rseq_cmpeqv_trystorev_storev_release(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -443,7 +447,7 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -527,7 +531,7 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -651,7 +655,7 @@ int rseq_cmpeqv_trymemcpy_storev_release(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
diff --git a/tools/testing/selftests/rseq/rseq-arm64.h b/tools/testing/selftests/rseq/rseq-arm64.h
index 2079f71e0ca2..b41a2a48e965 100644
--- a/tools/testing/selftests/rseq/rseq-arm64.h
+++ b/tools/testing/selftests/rseq/rseq-arm64.h
@@ -82,13 +82,16 @@ do { \
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, start_ip, \
post_commit_offset, abort_ip) \
- " .pushsection __rseq_table, \"aw\"\n" \
+ " .pushsection __rseq_cs, \"aw\"\n" \
" .balign 32\n" \
__rseq_str(label) ":\n" \
" .long " __rseq_str(version) ", " __rseq_str(flags) "\n" \
" .quad " __rseq_str(start_ip) ", " \
__rseq_str(post_commit_offset) ", " \
__rseq_str(abort_ip) "\n" \
+ " .popsection\n\t" \
+ " .pushsection __rseq_cs_ptr_array, \"aw\"\n" \
+ " .quad " __rseq_str(label) "b\n" \
" .popsection\n"
#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
@@ -99,8 +102,8 @@ do { \
* Exit points of a rseq critical section consist of all instructions outside
* of the critical section where a critical section can either branch to or
* reach through the normal course of its execution. The abort IP and the
- * post-commit IP are already part of the __rseq_table section and should not
- * be explicitly defined as additional exit points. Knowing all exit points is
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
* useful to assist debuggers stepping over the critical section.
*/
#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
diff --git a/tools/testing/selftests/rseq/rseq-mips.h b/tools/testing/selftests/rseq/rseq-mips.h
index 25d10ff54769..fe3eabcdcbe5 100644
--- a/tools/testing/selftests/rseq/rseq-mips.h
+++ b/tools/testing/selftests/rseq/rseq-mips.h
@@ -54,26 +54,30 @@ do { \
# error unsupported _MIPS_SZLONG
#endif
-#define __RSEQ_ASM_DEFINE_TABLE(version, flags, start_ip, \
+#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, start_ip, \
post_commit_offset, abort_ip) \
- ".pushsection __rseq_table, \"aw\"\n\t" \
+ ".pushsection __rseq_cs, \"aw\"\n\t" \
".balign 32\n\t" \
+ __rseq_str(label) ":\n\t" \
".word " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
LONG " " U32_U64_PAD(__rseq_str(start_ip)) "\n\t" \
LONG " " U32_U64_PAD(__rseq_str(post_commit_offset)) "\n\t" \
LONG " " U32_U64_PAD(__rseq_str(abort_ip)) "\n\t" \
+ ".popsection\n\t" \
+ ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
+ LONG " " U32_U64_PAD(__rseq_str(label) "b") "\n\t" \
".popsection\n\t"
-#define RSEQ_ASM_DEFINE_TABLE(start_ip, post_commit_ip, abort_ip) \
- __RSEQ_ASM_DEFINE_TABLE(0x0, 0x0, start_ip, \
+#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
+ __RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
(post_commit_ip - start_ip), abort_ip)
/*
* Exit points of a rseq critical section consist of all instructions outside
* of the critical section where a critical section can either branch to or
* reach through the normal course of its execution. The abort IP and the
- * post-commit IP are already part of the __rseq_table section and should not
- * be explicitly defined as additional exit points. Knowing all exit points is
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
* useful to assist debuggers stepping over the critical section.
*/
#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
@@ -127,7 +131,7 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -192,7 +196,7 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -261,7 +265,7 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
#endif
@@ -316,7 +320,7 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -389,7 +393,7 @@ int rseq_cmpeqv_trystorev_storev_release(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -463,7 +467,7 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -543,7 +547,7 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
@@ -664,7 +668,7 @@ int rseq_cmpeqv_trymemcpy_storev_release(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
- RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_TABLE(9, 1f, 2f, 4f) /* start, commit, abort */
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
#ifdef RSEQ_COMPARE_TWICE
RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
diff --git a/tools/testing/selftests/rseq/rseq-ppc.h b/tools/testing/selftests/rseq/rseq-ppc.h
index 24f95649d71e..9df18487fa9f 100644
--- a/tools/testing/selftests/rseq/rseq-ppc.h
+++ b/tools/testing/selftests/rseq/rseq-ppc.h
@@ -33,8 +33,8 @@ do { \
#else /* !RSEQ_SKIP_FASTPATH */
/*
- * The __rseq_table section can be used by debuggers to better handle
- * single-stepping through the restartable critical sections.
+ * The __rseq_cs_ptr_array and __rseq_cs sections can be used by debuggers to
+ * better handle single-stepping through the restartable critical sections.
*/
#ifdef __PPC64__
@@ -46,11 +46,14 @@ do { \
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
start_ip, post_commit_offset, abort_ip) \
- ".pushsection __rseq_table, \"aw\"\n\t" \
+ ".pushsection __rseq_cs, \"aw\"\n\t" \
".balign 32\n\t" \
__rseq_str(label) ":\n\t" \
".long " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
".quad " __rseq_str(start_ip) ", " __rseq_str(post_commit_offset) ", " __rseq_str(abort_ip) "\n\t" \
+ ".popsection\n\t" \
+ ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
+ ".quad " __rseq_str(label) "b\n\t" \
".popsection\n\t"
#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
@@ -67,8 +70,8 @@ do { \
* Exit points of a rseq critical section consist of all instructions outside
* of the critical section where a critical section can either branch to or
* reach through the normal course of its execution. The abort IP and the
- * post-commit IP are already part of the __rseq_table section and should not
- * be explicitly defined as additional exit points. Knowing all exit points is
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
* useful to assist debuggers stepping over the critical section.
*/
#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
@@ -85,20 +88,23 @@ do { \
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
start_ip, post_commit_offset, abort_ip) \
- ".pushsection __rseq_table, \"aw\"\n\t" \
+ ".pushsection __rseq_cs, \"aw\"\n\t" \
".balign 32\n\t" \
__rseq_str(label) ":\n\t" \
".long " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
/* 32-bit only supported on BE */ \
".long 0x0, " __rseq_str(start_ip) ", 0x0, " __rseq_str(post_commit_offset) ", 0x0, " __rseq_str(abort_ip) "\n\t" \
+ ".popsection\n\t" \
+ ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
+ ".long 0x0, " __rseq_str(label) "b\n\t" \
".popsection\n\t"
/*
* Exit points of a rseq critical section consist of all instructions outside
* of the critical section where a critical section can either branch to or
* reach through the normal course of its execution. The abort IP and the
- * post-commit IP are already part of the __rseq_table section and should not
- * be explicitly defined as additional exit points. Knowing all exit points is
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
* useful to assist debuggers stepping over the critical section.
*/
#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
diff --git a/tools/testing/selftests/rseq/rseq-s390.h b/tools/testing/selftests/rseq/rseq-s390.h
index b8b5b6f900af..fbb97815d71c 100644
--- a/tools/testing/selftests/rseq/rseq-s390.h
+++ b/tools/testing/selftests/rseq/rseq-s390.h
@@ -37,19 +37,22 @@ do { \
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
start_ip, post_commit_offset, abort_ip) \
- ".pushsection __rseq_table, \"aw\"\n\t" \
+ ".pushsection __rseq_cs, \"aw\"\n\t" \
".balign 32\n\t" \
__rseq_str(label) ":\n\t" \
".long " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
".quad " __rseq_str(start_ip) ", " __rseq_str(post_commit_offset) ", " __rseq_str(abort_ip) "\n\t" \
+ ".popsection\n\t" \
+ ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
+ ".quad " __rseq_str(label) "b\n\t" \
".popsection\n\t"
/*
* Exit points of a rseq critical section consist of all instructions outside
* of the critical section where a critical section can either branch to or
* reach through the normal course of its execution. The abort IP and the
- * post-commit IP are already part of the __rseq_table section and should not
- * be explicitly defined as additional exit points. Knowing all exit points is
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
* useful to assist debuggers stepping over the critical section.
*/
#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
@@ -61,19 +64,22 @@ do { \
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
start_ip, post_commit_offset, abort_ip) \
- ".pushsection __rseq_table, \"aw\"\n\t" \
+ ".pushsection __rseq_cs, \"aw\"\n\t" \
".balign 32\n\t" \
__rseq_str(label) ":\n\t" \
".long " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
".long 0x0, " __rseq_str(start_ip) ", 0x0, " __rseq_str(post_commit_offset) ", 0x0, " __rseq_str(abort_ip) "\n\t" \
+ ".popsection\n\t" \
+ ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
+ ".long 0x0, " __rseq_str(label) "b\n\t" \
".popsection\n\t"
/*
* Exit points of a rseq critical section consist of all instructions outside
* of the critical section where a critical section can either branch to or
* reach through the normal course of its execution. The abort IP and the
- * post-commit IP are already part of the __rseq_table section and should not
- * be explicitly defined as additional exit points. Knowing all exit points is
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
* useful to assist debuggers stepping over the critical section.
*/
#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
diff --git a/tools/testing/selftests/rseq/rseq-x86.h b/tools/testing/selftests/rseq/rseq-x86.h
index 5857570d1c51..2d4887b5d3f0 100644
--- a/tools/testing/selftests/rseq/rseq-x86.h
+++ b/tools/testing/selftests/rseq/rseq-x86.h
@@ -37,13 +37,17 @@ do { \
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
start_ip, post_commit_offset, abort_ip) \
- ".pushsection __rseq_table, \"aw\"\n\t" \
+ ".pushsection __rseq_cs, \"aw\"\n\t" \
".balign 32\n\t" \
__rseq_str(label) ":\n\t" \
".long " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
".quad " __rseq_str(start_ip) ", " __rseq_str(post_commit_offset) ", " __rseq_str(abort_ip) "\n\t" \
+ ".popsection\n\t" \
+ ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
+ ".quad " __rseq_str(label) "b\n\t" \
".popsection\n\t"
+
#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
__RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
(post_commit_ip - start_ip), abort_ip)
@@ -52,8 +56,8 @@ do { \
* Exit points of a rseq critical section consist of all instructions outside
* of the critical section where a critical section can either branch to or
* reach through the normal course of its execution. The abort IP and the
- * post-commit IP are already part of the __rseq_table section and should not
- * be explicitly defined as additional exit points. Knowing all exit points is
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
* useful to assist debuggers stepping over the critical section.
*/
#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
@@ -562,11 +566,14 @@ do { \
*/
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
start_ip, post_commit_offset, abort_ip) \
- ".pushsection __rseq_table, \"aw\"\n\t" \
+ ".pushsection __rseq_cs, \"aw\"\n\t" \
".balign 32\n\t" \
__rseq_str(label) ":\n\t" \
".long " __rseq_str(version) ", " __rseq_str(flags) "\n\t" \
".long " __rseq_str(start_ip) ", 0x0, " __rseq_str(post_commit_offset) ", 0x0, " __rseq_str(abort_ip) ", 0x0\n\t" \
+ ".popsection\n\t" \
+ ".pushsection __rseq_cs_ptr_array, \"aw\"\n\t" \
+ ".long " __rseq_str(label) "b, 0x0\n\t" \
".popsection\n\t"
#define RSEQ_ASM_DEFINE_TABLE(label, start_ip, post_commit_ip, abort_ip) \
@@ -577,8 +584,8 @@ do { \
* Exit points of a rseq critical section consist of all instructions outside
* of the critical section where a critical section can either branch to or
* reach through the normal course of its execution. The abort IP and the
- * post-commit IP are already part of the __rseq_table section and should not
- * be explicitly defined as additional exit points. Knowing all exit points is
+ * post-commit IP are already part of the __rseq_cs section and should not be
+ * explicitly defined as additional exit points. Knowing all exit points is
* useful to assist debuggers stepping over the critical section.
*/
#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
--
2.11.0
^ permalink raw reply related
* [RFC PATCH 1/2] rseq/selftests: Add __rseq_exit_point_array section for debuggers
From: Mathieu Desnoyers @ 2019-03-14 19:03 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk
In-Reply-To: <20190314190346.24744-1-mathieu.desnoyers@efficios.com>
Knowing all exit points is useful to assist debuggers stepping over the
rseq critical sections without requiring them to disassemble the content
of the critical section to figure out the exit points.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Joel Fernandes <joelaf@google.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Dave Watson <davejwatson@fb.com>
CC: Will Deacon <will.deacon@arm.com>
CC: Shuah Khan <shuah@kernel.org>
CC: Andi Kleen <andi@firstfloor.org>
CC: linux-kselftest@vger.kernel.org
CC: "H . Peter Anvin" <hpa@zytor.com>
CC: Chris Lameter <cl@linux.com>
CC: Russell King <linux@arm.linux.org.uk>
CC: Michael Kerrisk <mtk.manpages@gmail.com>
CC: "Paul E . McKenney" <paulmck@linux.vnet.ibm.com>
CC: Paul Turner <pjt@google.com>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Josh Triplett <josh@joshtriplett.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ben Maurer <bmaurer@fb.com>
CC: linux-api@vger.kernel.org
CC: Andy Lutomirski <luto@amacapital.net>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Linus Torvalds <torvalds@linux-foundation.org>
---
tools/testing/selftests/rseq/rseq-arm.h | 52 +++++++++++++++++
tools/testing/selftests/rseq/rseq-arm64.h | 52 +++++++++++++++++
tools/testing/selftests/rseq/rseq-mips.h | 53 +++++++++++++++++
tools/testing/selftests/rseq/rseq-ppc.h | 66 ++++++++++++++++++++++
tools/testing/selftests/rseq/rseq-s390.h | 55 ++++++++++++++++++
tools/testing/selftests/rseq/rseq-x86.h | 94 +++++++++++++++++++++++++++++++
6 files changed, 372 insertions(+)
diff --git a/tools/testing/selftests/rseq/rseq-arm.h b/tools/testing/selftests/rseq/rseq-arm.h
index 3cea19877227..17e8d231943a 100644
--- a/tools/testing/selftests/rseq/rseq-arm.h
+++ b/tools/testing/selftests/rseq/rseq-arm.h
@@ -42,6 +42,19 @@ do { \
__RSEQ_ASM_DEFINE_TABLE(0x0, 0x0, start_ip, \
(post_commit_ip - start_ip), abort_ip)
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_table section and should not
+ * be explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
+ ".word " __rseq_str(start_ip) ", 0x0, " __rseq_str(exit_ip) ", 0x0\n\t" \
+ ".popsection\n\t"
+
#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
RSEQ_INJECT_ASM(1) \
"adr r0, " __rseq_str(cs_label) "\n\t" \
@@ -87,6 +100,11 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -149,6 +167,11 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -215,6 +238,9 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -267,6 +293,11 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -337,6 +368,11 @@ int rseq_cmpeqv_trystorev_storev_release(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -408,6 +444,12 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error3])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -486,6 +528,11 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
"str %[src], %[rseq_scratch0]\n\t"
"str %[dst], %[rseq_scratch1]\n\t"
"str %[len], %[rseq_scratch2]\n\t"
@@ -605,6 +652,11 @@ int rseq_cmpeqv_trymemcpy_storev_release(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
"str %[src], %[rseq_scratch0]\n\t"
"str %[dst], %[rseq_scratch1]\n\t"
"str %[len], %[rseq_scratch2]\n\t"
diff --git a/tools/testing/selftests/rseq/rseq-arm64.h b/tools/testing/selftests/rseq/rseq-arm64.h
index 954f34671ca6..2079f71e0ca2 100644
--- a/tools/testing/selftests/rseq/rseq-arm64.h
+++ b/tools/testing/selftests/rseq/rseq-arm64.h
@@ -95,6 +95,19 @@ do { \
__RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
(post_commit_ip - start_ip), abort_ip)
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_table section and should not
+ * be explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ " .pushsection __rseq_exit_point_array, \"aw\"\n" \
+ " .quad " __rseq_str(start_ip) ", " __rseq_str(exit_ip) "\n" \
+ " .popsection\n"
+
#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
RSEQ_INJECT_ASM(1) \
" adrp " RSEQ_ASM_TMP_REG ", " __rseq_str(cs_label) "\n" \
@@ -182,6 +195,11 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error2])
+#endif
RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
@@ -231,6 +249,11 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error2])
+#endif
RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
@@ -282,6 +305,9 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error1])
+#endif
RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
@@ -325,6 +351,11 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error2])
+#endif
RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
@@ -379,6 +410,11 @@ int rseq_cmpeqv_trystorev_storev_release(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error2])
+#endif
RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
@@ -433,6 +469,12 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error2])
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error3])
+#endif
RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
@@ -490,6 +532,11 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error2])
+#endif
RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
@@ -545,6 +592,11 @@ int rseq_cmpeqv_trymemcpy_storev_release(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1, 2f, 3f, 4f)
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(2f, %l[error2])
+#endif
RSEQ_ASM_STORE_RSEQ_CS(2, 1b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
RSEQ_INJECT_ASM(3)
diff --git a/tools/testing/selftests/rseq/rseq-mips.h b/tools/testing/selftests/rseq/rseq-mips.h
index 7f48ecf46994..25d10ff54769 100644
--- a/tools/testing/selftests/rseq/rseq-mips.h
+++ b/tools/testing/selftests/rseq/rseq-mips.h
@@ -68,6 +68,20 @@ do { \
__RSEQ_ASM_DEFINE_TABLE(0x0, 0x0, start_ip, \
(post_commit_ip - start_ip), abort_ip)
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_table section and should not
+ * be explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
+ LONG " " U32_U64_PAD(__rseq_str(start_ip)) "\n\t" \
+ LONG " " U32_U64_PAD(__rseq_str(exit_ip)) "\n\t" \
+ ".popsection\n\t"
+
#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
RSEQ_INJECT_ASM(1) \
LONG_LA " $4, " __rseq_str(cs_label) "\n\t" \
@@ -114,6 +128,11 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -174,6 +193,11 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -238,6 +262,9 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -290,6 +317,11 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -358,6 +390,11 @@ int rseq_cmpeqv_trystorev_storev_release(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -427,6 +464,12 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error3])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3f, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -501,6 +544,11 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
LONG_S " %[src], %[rseq_scratch0]\n\t"
LONG_S " %[dst], %[rseq_scratch1]\n\t"
LONG_S " %[len], %[rseq_scratch2]\n\t"
@@ -617,6 +665,11 @@ int rseq_cmpeqv_trymemcpy_storev_release(intptr_t *v, intptr_t expect,
rseq_workaround_gcc_asm_size_guess();
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
LONG_S " %[src], %[rseq_scratch0]\n\t"
LONG_S " %[dst], %[rseq_scratch1]\n\t"
LONG_S " %[len], %[rseq_scratch2]\n\t"
diff --git a/tools/testing/selftests/rseq/rseq-ppc.h b/tools/testing/selftests/rseq/rseq-ppc.h
index 52630c9f42be..24f95649d71e 100644
--- a/tools/testing/selftests/rseq/rseq-ppc.h
+++ b/tools/testing/selftests/rseq/rseq-ppc.h
@@ -63,6 +63,19 @@ do { \
"std %%r17, %[" __rseq_str(rseq_cs) "]\n\t" \
__rseq_str(label) ":\n\t"
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_table section and should not
+ * be explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
+ ".quad " __rseq_str(start_ip) ", " __rseq_str(exit_ip) "\n\t" \
+ ".popsection\n\t"
+
#else /* #ifdef __PPC64__ */
#define STORE_WORD "stw "
@@ -80,6 +93,20 @@ do { \
".long 0x0, " __rseq_str(start_ip) ", 0x0, " __rseq_str(post_commit_offset) ", 0x0, " __rseq_str(abort_ip) "\n\t" \
".popsection\n\t"
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_table section and should not
+ * be explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
+ /* 32-bit only supported on BE */ \
+ ".long 0x0, " __rseq_str(start_ip) ", 0x0, " __rseq_str(exit_ip) "\n\t" \
+ ".popsection\n\t"
+
#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
RSEQ_INJECT_ASM(1) \
"lis %%r17, (" __rseq_str(cs_label) ")@ha\n\t" \
@@ -169,6 +196,11 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
/* cmp cpuid */
@@ -224,6 +256,11 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
/* cmp cpuid */
@@ -286,6 +323,9 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
/* cmp cpuid */
@@ -337,6 +377,11 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
/* cmp cpuid */
@@ -400,6 +445,11 @@ int rseq_cmpeqv_trystorev_storev_release(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
/* cmp cpuid */
@@ -465,6 +515,12 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error3])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
/* cmp cpuid */
@@ -532,6 +588,11 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* setup for mempcy */
"mr %%r19, %[len]\n\t"
"mr %%r20, %[src]\n\t"
@@ -601,6 +662,11 @@ int rseq_cmpeqv_trymemcpy_storev_release(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* setup for mempcy */
"mr %%r19, %[len]\n\t"
"mr %%r20, %[src]\n\t"
diff --git a/tools/testing/selftests/rseq/rseq-s390.h b/tools/testing/selftests/rseq/rseq-s390.h
index 1069e85258ce..b8b5b6f900af 100644
--- a/tools/testing/selftests/rseq/rseq-s390.h
+++ b/tools/testing/selftests/rseq/rseq-s390.h
@@ -44,6 +44,19 @@ do { \
".quad " __rseq_str(start_ip) ", " __rseq_str(post_commit_offset) ", " __rseq_str(abort_ip) "\n\t" \
".popsection\n\t"
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_table section and should not
+ * be explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
+ ".quad " __rseq_str(start_ip) ", " __rseq_str(exit_ip) "\n\t" \
+ ".popsection\n\t"
+
#elif __s390__
#define __RSEQ_ASM_DEFINE_TABLE(label, version, flags, \
@@ -55,6 +68,19 @@ do { \
".long 0x0, " __rseq_str(start_ip) ", 0x0, " __rseq_str(post_commit_offset) ", 0x0, " __rseq_str(abort_ip) "\n\t" \
".popsection\n\t"
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_table section and should not
+ * be explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
+ ".long 0x0, " __rseq_str(start_ip) ", 0x0, " __rseq_str(exit_ip) "\n\t" \
+ ".popsection\n\t"
+
#define LONG_L "l"
#define LONG_S "st"
#define LONG_LT_R "ltr"
@@ -102,6 +128,11 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -160,6 +191,11 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -220,6 +256,9 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -268,6 +307,11 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -339,6 +383,12 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error3])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -407,6 +457,11 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
LONG_S " %[src], %[rseq_scratch0]\n\t"
LONG_S " %[dst], %[rseq_scratch1]\n\t"
LONG_S " %[len], %[rseq_scratch2]\n\t"
diff --git a/tools/testing/selftests/rseq/rseq-x86.h b/tools/testing/selftests/rseq/rseq-x86.h
index 089410a314e9..5857570d1c51 100644
--- a/tools/testing/selftests/rseq/rseq-x86.h
+++ b/tools/testing/selftests/rseq/rseq-x86.h
@@ -48,6 +48,19 @@ do { \
__RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
(post_commit_ip - start_ip), abort_ip)
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_table section and should not
+ * be explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
+ ".quad " __rseq_str(start_ip) ", " __rseq_str(exit_ip) "\n\t" \
+ ".popsection\n\t"
+
#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
RSEQ_INJECT_ASM(1) \
"leaq " __rseq_str(cs_label) "(%%rip), %%rax\n\t" \
@@ -83,6 +96,11 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -140,6 +158,11 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -199,6 +222,9 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -244,6 +270,11 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -314,6 +345,12 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error3])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -381,6 +418,11 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
"movq %[src], %[rseq_scratch0]\n\t"
"movq %[dst], %[rseq_scratch1]\n\t"
"movq %[len], %[rseq_scratch2]\n\t"
@@ -531,6 +573,19 @@ do { \
__RSEQ_ASM_DEFINE_TABLE(label, 0x0, 0x0, start_ip, \
(post_commit_ip - start_ip), abort_ip)
+/*
+ * Exit points of a rseq critical section consist of all instructions outside
+ * of the critical section where a critical section can either branch to or
+ * reach through the normal course of its execution. The abort IP and the
+ * post-commit IP are already part of the __rseq_table section and should not
+ * be explicitly defined as additional exit points. Knowing all exit points is
+ * useful to assist debuggers stepping over the critical section.
+ */
+#define RSEQ_ASM_DEFINE_EXIT_POINT(start_ip, exit_ip) \
+ ".pushsection __rseq_exit_point_array, \"aw\"\n\t" \
+ ".long " __rseq_str(start_ip) ", 0x0, " __rseq_str(exit_ip) ", 0x0\n\t" \
+ ".popsection\n\t"
+
#define RSEQ_ASM_STORE_RSEQ_CS(label, cs_label, rseq_cs) \
RSEQ_INJECT_ASM(1) \
"movl $" __rseq_str(cs_label) ", %[rseq_cs]\n\t" \
@@ -565,6 +620,11 @@ int rseq_cmpeqv_storev(intptr_t *v, intptr_t expect, intptr_t newv, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -622,6 +682,11 @@ int rseq_cmpnev_storeoffp_load(intptr_t *v, intptr_t expectnot,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -681,6 +746,9 @@ int rseq_addv(intptr_t *v, intptr_t count, int cpu)
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -726,6 +794,11 @@ int rseq_cmpeqv_trystorev_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -788,6 +861,11 @@ int rseq_cmpeqv_trystorev_storev_release(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -853,6 +931,12 @@ int rseq_cmpeqv_cmpeqv_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error3])
+#endif
/* Start rseq by storing table entry pointer into rseq_cs. */
RSEQ_ASM_STORE_RSEQ_CS(1, 3b, rseq_cs)
RSEQ_ASM_CMP_CPU_ID(cpu_id, current_cpu_id, 4f)
@@ -922,6 +1006,11 @@ int rseq_cmpeqv_trymemcpy_storev(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
"movl %[src], %[rseq_scratch0]\n\t"
"movl %[dst], %[rseq_scratch1]\n\t"
"movl %[len], %[rseq_scratch2]\n\t"
@@ -1030,6 +1119,11 @@ int rseq_cmpeqv_trymemcpy_storev_release(intptr_t *v, intptr_t expect,
__asm__ __volatile__ goto (
RSEQ_ASM_DEFINE_TABLE(3, 1f, 2f, 4f) /* start, commit, abort */
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[cmpfail])
+#ifdef RSEQ_COMPARE_TWICE
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error1])
+ RSEQ_ASM_DEFINE_EXIT_POINT(1f, %l[error2])
+#endif
"movl %[src], %[rseq_scratch0]\n\t"
"movl %[dst], %[rseq_scratch1]\n\t"
"movl %[len], %[rseq_scratch2]\n\t"
--
2.11.0
^ permalink raw reply related
* [RFC PATCH 0/2] rseq/selftests: debugger single-stepping helper sections
From: Mathieu Desnoyers @ 2019-03-14 19:03 UTC (permalink / raw)
To: Thomas Gleixner
Cc: linux-kernel, linux-api, Peter Zijlstra, Paul E . McKenney,
Boqun Feng, Andy Lutomirski, Dave Watson, Paul Turner,
Andrew Morton, Russell King, Ingo Molnar, H . Peter Anvin,
Andi Kleen, Chris Lameter, Ben Maurer, Steven Rostedt,
Josh Triplett, Linus Torvalds, Catalin Marinas, Will Deacon,
Michael Kerrisk
Hi,
Those RFC patches improves the rseq selftests to properly expose data
sections helping debuggers to step over rseq critical sections.
* Add a new __rseq_exit_point_array section describing all rseq critical
section exit points so debuggers can properly step over those. This is
based on discussions with gdb and DynamoRIO developers.
* The __rseq_table section contains structures that are aligned on 32
bytes, and in some cases the entries may not be aligned with the
beginning of the section. Rename this section to __rseq_cs, and create
a new __rseq_cs_ptr_array section containing packed 64-bit pointers to
those __rseq_cs entries.
Thanks,
Mathieu
Mathieu Desnoyers (2):
rseq/selftests: Add __rseq_exit_point_array section for debuggers
rseq/selftests: Introduce __rseq_cs_ptr_array, rename __rseq_table to
__rseq_cs
tools/testing/selftests/rseq/rseq-arm.h | 80 +++++++++++++++++++----
tools/testing/selftests/rseq/rseq-arm64.h | 57 +++++++++++++++-
tools/testing/selftests/rseq/rseq-mips.h | 81 +++++++++++++++++++----
tools/testing/selftests/rseq/rseq-ppc.h | 80 +++++++++++++++++++++--
tools/testing/selftests/rseq/rseq-s390.h | 65 +++++++++++++++++-
tools/testing/selftests/rseq/rseq-x86.h | 105 +++++++++++++++++++++++++++++-
6 files changed, 435 insertions(+), 33 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [PATCH] x86: Deprecate a.out support
From: Miguel Ojeda @ 2019-03-14 18:38 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Torvalds, Arnd Bergmann, Måns Rullgård,
Matt Turner, Borislav Petkov, Alan Cox, Matthew Wilcox, Jann Horn,
Al Viro, Thomas Gleixner, kernel list, linux-fsdevel,
the arch/x86 maintainers, Linux API, Andrew Morton,
Richard Weinberger, Anton Ivanov, linux-alpha, linux-m68k
In-Reply-To: <CAMuHMdU76V=p8GLqVdChx+Sa_aPZ9BMCnnzmzWonqEq4Xe+LEQ@mail.gmail.com>
On Tue, Mar 12, 2019 at 9:46 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Yeah, the alphas on the server side, powering AltaVista, are also long
> gone...
As usual with these things, people can still use older Linux releases
for a very long time. If they really need it (e.g. commercially), they
have the reference code and can bring it up to speed again relatively
easily. It is not as if they have no way to submit it back again.
Cheers,
Miguel
^ permalink raw reply
* Re: [PATCH] y2038: fix socket.h header inclusion
From: Florian Weimer @ 2019-03-14 18:37 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David S . Miller, Deepa Dinamani, Willem de Bruijn, linux-alpha,
linux-arch, linux-mips, linux-parisc, sparclinux, Laura Abbott,
netdev, linux-kernel, linux-api
In-Reply-To: <20190311153857.563743-1-arnd@arndb.de>
* Arnd Bergmann:
> diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
> index 0d0fddb7e738..976e89b116e5 100644
> --- a/arch/alpha/include/uapi/asm/socket.h
> +++ b/arch/alpha/include/uapi/asm/socket.h
> @@ -2,8 +2,8 @@
> #ifndef _UAPI_ASM_SOCKET_H
> #define _UAPI_ASM_SOCKET_H
>
> +#include <linux/posix_types.h>
> #include <asm/sockios.h>
> -#include <asm/bitsperlong.h>
This breaks POSIX conformance in glibc because the
<linux/posix_types.h> header is not namespace clean. It contains the
identifiers fds_bits and val:
unsigned long fds_bits[__FD_SETSIZE / (8 * sizeof(long))];
int val[2];
We could duplicate some of the SO_* constants for POSIX mode in glibc,
but it would be nice to avoid that.
Is there a different way of fixing this on the kernel side that avoids
including <linux/posix_types.h>?
^ permalink raw reply
* Re: [PATCH v7 02/15] sched/core: uclamp: Enforce last task UCLAMP_MAX
From: Patrick Bellasi @ 2019-03-14 17:06 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Peter Zijlstra, LKML, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <CAJuCfpH2QPgB3D+qsbF7gzDhrD4Y6oo3Q-b8iwtUxZe-dDWR7Q@mail.gmail.com>
On 13-Mar 17:29, Suren Baghdasaryan wrote:
> On Wed, Mar 13, 2019 at 9:16 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
> >
> > On 13-Mar 15:12, Peter Zijlstra wrote:
> > > On Fri, Feb 08, 2019 at 10:05:41AM +0000, Patrick Bellasi wrote:
> > > > +static inline void uclamp_idle_reset(struct rq *rq, unsigned int clamp_id,
> > > > + unsigned int clamp_value)
> > > > +{
> > > > + /* Reset max-clamp retention only on idle exit */
> > > > + if (!(rq->uclamp_flags & UCLAMP_FLAG_IDLE))
> > > > + return;
> > > > +
> > > > + WRITE_ONCE(rq->uclamp[clamp_id].value, clamp_value);
> > > > +
> > > > + /*
> > > > + * This function is called for both UCLAMP_MIN (before) and UCLAMP_MAX
> > > > + * (after). The idle flag is reset only the second time, when we know
> > > > + * that UCLAMP_MIN has been already updated.
> > >
> > > Why do we care? That is, what is this comment trying to tell us.
> >
> > Right, the code is clear enough, I'll remove this comment.
>
> It would be probably even clearer if rq->uclamp_flags &=
> ~UCLAMP_FLAG_IDLE is done from inside uclamp_rq_inc after
> uclamp_rq_inc_id for both clamps is called.
Good point! I'll move it there to have something like:
---8<---
static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
{
unsigned int clamp_id;
if (unlikely(!p->sched_class->uclamp_enabled))
return;
for (clamp_id = 0; clamp_id < UCLAMP_CNT; ++clamp_id)
uclamp_rq_inc_id(p, rq, clamp_id);
/* Reset clamp holding when we have at least one RUNNABLE task */
if (rq->uclamp_flags & UCLAMP_FLAG_IDLE)
rq->uclamp_flags &= ~UCLAMP_FLAG_IDLE;
}
---8<---
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Suren Baghdasaryan @ 2019-03-14 16:39 UTC (permalink / raw)
To: Patrick Bellasi
Cc: LKML, linux-pm, linux-api, Ingo Molnar, Peter Zijlstra, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <20190314154052.345v74snhtc4cqbe@e110439-lin>
On Thu, Mar 14, 2019 at 8:41 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
>
> On 14-Mar 08:29, Suren Baghdasaryan wrote:
> > On Thu, Mar 14, 2019 at 7:46 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
> > > On 13-Mar 14:32, Suren Baghdasaryan wrote:
> > > > On Fri, Feb 8, 2019 at 2:06 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
>
> [...]
>
> > > > > The rq::uclamp::bucket[clamp_id][] array is scanned every time we need
> > > > > to find a new MAX aggregated clamp value for a clamp_id. This operation
> > > > > is required only when we dequeue the last task of a clamp bucket
> > > > > tracking the current MAX aggregated clamp value. In these cases, the CPU
> > > > > is either entering IDLE or going to schedule a less boosted or more
> > > > > clamped task.
>
> The following:
>
> > > > > The expected number of different clamp values, configured at build time,
> > > > > is small enough to fit the full unordered array into a single cache
> > > > > line.
>
> will read:
>
> The expected number of different clamp values, configured at build time,
> is small enough to fit the full unordered array into a single cache
> line for the default UCLAMP_BUCKETS configuration of 7 buckets.
I think keeping default to be 5 is good. As you mentioned it's a nice
round number and keeping it at the minimum also hints that this is not
a free resource and the more buckets you use the more you pay.
Documentation might say "to fit the full unordered array into a single
cache line for configurations of up to 7 buckets".
> [...]
>
> > Got it. From reading the documentation at the beginning my impression
> > was that whatever value I choose within allowed 5-20 range it would
> > still fit in a cache line. To disambiguate it might be worse
> > mentioning that this is true for the default value or for values up to
> > 7. Thanks!
>
> Right, I hope the above proposed change helps to clarify that.
>
> --
> #include <best/regards.h>
>
> Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 12/15] sched/core: uclamp: Propagate parent clamps
From: Suren Baghdasaryan @ 2019-03-14 16:17 UTC (permalink / raw)
To: Patrick Bellasi
Cc: LKML, linux-pm, linux-api, Ingo Molnar, Peter Zijlstra, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <20190208100554.32196-13-patrick.bellasi@arm.com>
On Fri, Feb 8, 2019 at 2:06 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
>
> In order to properly support hierarchical resources control, the cgroup
> delegation model requires that attribute writes from a child group never
> fail but still are (potentially) constrained based on parent's assigned
> resources. This requires to properly propagate and aggregate parent
> attributes down to its descendants.
>
> Let's implement this mechanism by adding a new "effective" clamp value
> for each task group. The effective clamp value is defined as the smaller
> value between the clamp value of a group and the effective clamp value
> of its parent. This is the actual clamp value enforced on tasks in a
> task group.
In patch 10 in this series you mentioned "b) do not enforce any
constraints and/or dependencies between the parent and its child
nodes"
This patch seems to change that behavior. If so, should it be documented?
> Since it can be interesting for userspace, e.g. system management
> software, to know exactly what the currently propagated/enforced
> configuration is, the effective clamp values are exposed to user-space
> by means of a new pair of read-only attributes
> cpu.util.{min,max}.effective.
>
> Signed-off-by: Patrick Bellasi <patrick.bellasi@arm.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Tejun Heo <tj@kernel.org>
>
> ---
> Changes in v7:
> Others:
> - ensure clamp values are not tunable at root cgroup level
> ---
> Documentation/admin-guide/cgroup-v2.rst | 19 ++++
> kernel/sched/core.c | 118 +++++++++++++++++++++++-
> 2 files changed, 133 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
> index 47710a77f4fa..7aad2435e961 100644
> --- a/Documentation/admin-guide/cgroup-v2.rst
> +++ b/Documentation/admin-guide/cgroup-v2.rst
> @@ -990,6 +990,16 @@ All time durations are in microseconds.
> values similar to the sched_setattr(2). This minimum utilization
> value is used to clamp the task specific minimum utilization clamp.
>
> + cpu.util.min.effective
> + A read-only single value file which exists on non-root cgroups and
> + reports minimum utilization clamp value currently enforced on a task
> + group.
> +
> + The actual minimum utilization in the range [0, 1024].
> +
> + This value can be lower then cpu.util.min in case a parent cgroup
> + allows only smaller minimum utilization values.
> +
> cpu.util.max
> A read-write single value file which exists on non-root cgroups.
> The default is "1024". i.e. no utilization capping
> @@ -1000,6 +1010,15 @@ All time durations are in microseconds.
> values similar to the sched_setattr(2). This maximum utilization
> value is used to clamp the task specific maximum utilization clamp.
>
> + cpu.util.max.effective
> + A read-only single value file which exists on non-root cgroups and
> + reports maximum utilization clamp value currently enforced on a task
> + group.
> +
> + The actual maximum utilization in the range [0, 1024].
> +
> + This value can be lower then cpu.util.max in case a parent cgroup
> + is enforcing a more restrictive clamping on max utilization.
>
>
> Memory
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 122ab069ade5..1e54517acd58 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -720,6 +720,18 @@ static void set_load_weight(struct task_struct *p, bool update_load)
> }
>
> #ifdef CONFIG_UCLAMP_TASK
> +/*
> + * Serializes updates of utilization clamp values
> + *
> + * The (slow-path) user-space triggers utilization clamp value updates which
> + * can require updates on (fast-path) scheduler's data structures used to
> + * support enqueue/dequeue operations.
> + * While the per-CPU rq lock protects fast-path update operations, user-space
> + * requests are serialized using a mutex to reduce the risk of conflicting
> + * updates or API abuses.
> + */
> +static DEFINE_MUTEX(uclamp_mutex);
> +
> /* Max allowed minimum utilization */
> unsigned int sysctl_sched_uclamp_util_min = SCHED_CAPACITY_SCALE;
>
> @@ -1127,6 +1139,8 @@ static void __init init_uclamp(void)
> unsigned int value;
> int cpu;
>
> + mutex_init(&uclamp_mutex);
> +
> for_each_possible_cpu(cpu) {
> memset(&cpu_rq(cpu)->uclamp, 0, sizeof(struct uclamp_rq));
> cpu_rq(cpu)->uclamp_flags = 0;
> @@ -6758,6 +6772,10 @@ static inline int alloc_uclamp_sched_group(struct task_group *tg,
> parent->uclamp[clamp_id].value;
> tg->uclamp[clamp_id].bucket_id =
> parent->uclamp[clamp_id].bucket_id;
> + tg->uclamp[clamp_id].effective.value =
> + parent->uclamp[clamp_id].effective.value;
> + tg->uclamp[clamp_id].effective.bucket_id =
> + parent->uclamp[clamp_id].effective.bucket_id;
> }
> #endif
>
> @@ -7011,6 +7029,53 @@ static void cpu_cgroup_attach(struct cgroup_taskset *tset)
> }
>
> #ifdef CONFIG_UCLAMP_TASK_GROUP
> +static void cpu_util_update_hier(struct cgroup_subsys_state *css,
s/cpu_util_update_hier/cpu_util_update_heir ?
> + unsigned int clamp_id, unsigned int bucket_id,
> + unsigned int value)
> +{
> + struct cgroup_subsys_state *top_css = css;
> + struct uclamp_se *uc_se, *uc_parent;
> +
> + css_for_each_descendant_pre(css, top_css) {
> + /*
> + * The first visited task group is top_css, which clamp value
> + * is the one passed as parameter. For descendent task
> + * groups we consider their current value.
> + */
> + uc_se = &css_tg(css)->uclamp[clamp_id];
> + if (css != top_css) {
> + value = uc_se->value;
> + bucket_id = uc_se->effective.bucket_id;
> + }
> + uc_parent = NULL;
> + if (css_tg(css)->parent)
> + uc_parent = &css_tg(css)->parent->uclamp[clamp_id];
> +
> + /*
> + * Skip the whole subtrees if the current effective clamp is
> + * already matching the TG's clamp value.
> + * In this case, all the subtrees already have top_value, or a
> + * more restrictive value, as effective clamp.
> + */
> + if (uc_se->effective.value == value &&
> + uc_parent && uc_parent->effective.value >= value) {
> + css = css_rightmost_descendant(css);
> + continue;
> + }
> +
> + /* Propagate the most restrictive effective value */
> + if (uc_parent && uc_parent->effective.value < value) {
> + value = uc_parent->effective.value;
> + bucket_id = uc_parent->effective.bucket_id;
> + }
> + if (uc_se->effective.value == value)
> + continue;
> +
> + uc_se->effective.value = value;
> + uc_se->effective.bucket_id = bucket_id;
> + }
> +}
> +
> static int cpu_util_min_write_u64(struct cgroup_subsys_state *css,
> struct cftype *cftype, u64 min_value)
> {
> @@ -7020,6 +7085,7 @@ static int cpu_util_min_write_u64(struct cgroup_subsys_state *css,
> if (min_value > SCHED_CAPACITY_SCALE)
> return -ERANGE;
>
> + mutex_lock(&uclamp_mutex);
> rcu_read_lock();
>
> tg = css_tg(css);
> @@ -7038,8 +7104,13 @@ static int cpu_util_min_write_u64(struct cgroup_subsys_state *css,
> tg->uclamp[UCLAMP_MIN].value = min_value;
> tg->uclamp[UCLAMP_MIN].bucket_id = uclamp_bucket_id(min_value);
>
> + /* Update effective clamps to track the most restrictive value */
> + cpu_util_update_hier(css, UCLAMP_MIN, tg->uclamp[UCLAMP_MIN].bucket_id,
> + min_value);
> +
> out:
> rcu_read_unlock();
> + mutex_unlock(&uclamp_mutex);
>
> return ret;
> }
> @@ -7053,6 +7124,7 @@ static int cpu_util_max_write_u64(struct cgroup_subsys_state *css,
> if (max_value > SCHED_CAPACITY_SCALE)
> return -ERANGE;
>
> + mutex_lock(&uclamp_mutex);
> rcu_read_lock();
>
> tg = css_tg(css);
> @@ -7071,21 +7143,29 @@ static int cpu_util_max_write_u64(struct cgroup_subsys_state *css,
> tg->uclamp[UCLAMP_MAX].value = max_value;
> tg->uclamp[UCLAMP_MAX].bucket_id = uclamp_bucket_id(max_value);
>
> + /* Update effective clamps to track the most restrictive value */
> + cpu_util_update_hier(css, UCLAMP_MAX, tg->uclamp[UCLAMP_MAX].bucket_id,
> + max_value);
> +
> out:
> rcu_read_unlock();
> + mutex_unlock(&uclamp_mutex);
>
> return ret;
> }
>
> static inline u64 cpu_uclamp_read(struct cgroup_subsys_state *css,
> - enum uclamp_id clamp_id)
> + enum uclamp_id clamp_id,
> + bool effective)
> {
> struct task_group *tg;
> u64 util_clamp;
>
> rcu_read_lock();
> tg = css_tg(css);
> - util_clamp = tg->uclamp[clamp_id].value;
> + util_clamp = effective
> + ? tg->uclamp[clamp_id].effective.value
> + : tg->uclamp[clamp_id].value;
> rcu_read_unlock();
>
> return util_clamp;
> @@ -7094,13 +7174,25 @@ static inline u64 cpu_uclamp_read(struct cgroup_subsys_state *css,
> static u64 cpu_util_min_read_u64(struct cgroup_subsys_state *css,
> struct cftype *cft)
> {
> - return cpu_uclamp_read(css, UCLAMP_MIN);
> + return cpu_uclamp_read(css, UCLAMP_MIN, false);
> }
>
> static u64 cpu_util_max_read_u64(struct cgroup_subsys_state *css,
> struct cftype *cft)
> {
> - return cpu_uclamp_read(css, UCLAMP_MAX);
> + return cpu_uclamp_read(css, UCLAMP_MAX, false);
> +}
> +
> +static u64 cpu_util_min_effective_read_u64(struct cgroup_subsys_state *css,
> + struct cftype *cft)
> +{
> + return cpu_uclamp_read(css, UCLAMP_MIN, true);
> +}
> +
> +static u64 cpu_util_max_effective_read_u64(struct cgroup_subsys_state *css,
> + struct cftype *cft)
> +{
> + return cpu_uclamp_read(css, UCLAMP_MAX, true);
> }
> #endif /* CONFIG_UCLAMP_TASK_GROUP */
>
> @@ -7448,11 +7540,19 @@ static struct cftype cpu_legacy_files[] = {
> .read_u64 = cpu_util_min_read_u64,
> .write_u64 = cpu_util_min_write_u64,
> },
> + {
> + .name = "util.min.effective",
> + .read_u64 = cpu_util_min_effective_read_u64,
> + },
> {
> .name = "util.max",
> .read_u64 = cpu_util_max_read_u64,
> .write_u64 = cpu_util_max_write_u64,
> },
> + {
> + .name = "util.max.effective",
> + .read_u64 = cpu_util_max_effective_read_u64,
> + },
> #endif
> { } /* Terminate */
> };
> @@ -7628,12 +7728,22 @@ static struct cftype cpu_files[] = {
> .read_u64 = cpu_util_min_read_u64,
> .write_u64 = cpu_util_min_write_u64,
> },
> + {
> + .name = "util.min.effective",
> + .flags = CFTYPE_NOT_ON_ROOT,
> + .read_u64 = cpu_util_min_effective_read_u64,
> + },
> {
> .name = "util.max",
> .flags = CFTYPE_NOT_ON_ROOT,
> .read_u64 = cpu_util_max_read_u64,
> .write_u64 = cpu_util_max_write_u64,
> },
> + {
> + .name = "util.max.effective",
> + .flags = CFTYPE_NOT_ON_ROOT,
> + .read_u64 = cpu_util_max_effective_read_u64,
> + },
> #endif
> { } /* terminate */
> };
> --
> 2.20.1
>
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 15:40 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: LKML, linux-pm, linux-api, Ingo Molnar, Peter Zijlstra, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <CAJuCfpFDVi7O+yqQ+F0DT3pRZN=BtRD8NyUH5Tys0VVkd9JAGg@mail.gmail.com>
On 14-Mar 08:29, Suren Baghdasaryan wrote:
> On Thu, Mar 14, 2019 at 7:46 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
> > On 13-Mar 14:32, Suren Baghdasaryan wrote:
> > > On Fri, Feb 8, 2019 at 2:06 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
[...]
> > > > The rq::uclamp::bucket[clamp_id][] array is scanned every time we need
> > > > to find a new MAX aggregated clamp value for a clamp_id. This operation
> > > > is required only when we dequeue the last task of a clamp bucket
> > > > tracking the current MAX aggregated clamp value. In these cases, the CPU
> > > > is either entering IDLE or going to schedule a less boosted or more
> > > > clamped task.
The following:
> > > > The expected number of different clamp values, configured at build time,
> > > > is small enough to fit the full unordered array into a single cache
> > > > line.
will read:
The expected number of different clamp values, configured at build time,
is small enough to fit the full unordered array into a single cache
line for the default UCLAMP_BUCKETS configuration of 7 buckets.
[...]
> Got it. From reading the documentation at the beginning my impression
> was that whatever value I choose within allowed 5-20 range it would
> still fit in a cache line. To disambiguate it might be worse
> mentioning that this is true for the default value or for values up to
> 7. Thanks!
Right, I hope the above proposed change helps to clarify that.
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Suren Baghdasaryan @ 2019-03-14 15:29 UTC (permalink / raw)
To: Patrick Bellasi
Cc: LKML, linux-pm, linux-api, Ingo Molnar, Peter Zijlstra, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <20190314144600.2ulpeipad7jbxyiy@e110439-lin>
On Thu, Mar 14, 2019 at 7:46 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
>
> On 13-Mar 14:32, Suren Baghdasaryan wrote:
> > On Fri, Feb 8, 2019 at 2:06 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
> > >
> > > Utilization clamping allows to clamp the CPU's utilization within a
> > > [util_min, util_max] range, depending on the set of RUNNABLE tasks on
> > > that CPU. Each task references two "clamp buckets" defining its minimum
> > > and maximum (util_{min,max}) utilization "clamp values". A CPU's clamp
> > > bucket is active if there is at least one RUNNABLE tasks enqueued on
> > > that CPU and refcounting that bucket.
> > >
> > > When a task is {en,de}queued {on,from} a rq, the set of active clamp
> > > buckets on that CPU can change. Since each clamp bucket enforces a
> > > different utilization clamp value, when the set of active clamp buckets
> > > changes, a new "aggregated" clamp value is computed for that CPU.
> > >
> > > Clamp values are always MAX aggregated for both util_min and util_max.
> > > This ensures that no tasks can affect the performance of other
> > > co-scheduled tasks which are more boosted (i.e. with higher util_min
> > > clamp) or less capped (i.e. with higher util_max clamp).
> > >
> > > Each task has a:
> > > task_struct::uclamp[clamp_id]::bucket_id
> > > to track the "bucket index" of the CPU's clamp bucket it refcounts while
> > > enqueued, for each clamp index (clamp_id).
> > >
> > > Each CPU's rq has a:
> > > rq::uclamp[clamp_id]::bucket[bucket_id].tasks
> > > to track how many tasks, currently RUNNABLE on that CPU, refcount each
> > > clamp bucket (bucket_id) of a clamp index (clamp_id).
> > >
> > > Each CPU's rq has also a:
> > > rq::uclamp[clamp_id]::bucket[bucket_id].value
> > > to track the clamp value of each clamp bucket (bucket_id) of a clamp
> > > index (clamp_id).
> > >
> > > The rq::uclamp::bucket[clamp_id][] array is scanned every time we need
> > > to find a new MAX aggregated clamp value for a clamp_id. This operation
> > > is required only when we dequeue the last task of a clamp bucket
> > > tracking the current MAX aggregated clamp value. In these cases, the CPU
> > > is either entering IDLE or going to schedule a less boosted or more
> > > clamped task.
> > > The expected number of different clamp values, configured at build time,
> > > is small enough to fit the full unordered array into a single cache
> > > line.
> >
> > I assume you are talking about "struct uclamp_rq uclamp[UCLAMP_CNT]"
> > here.
>
> No, I'm talking about the rq::uclamp::bucket[clamp_id][], which is an
> array of:
>
> struct uclamp_bucket {
> unsigned long value : bits_per(SCHED_CAPACITY_SCALE);
> unsigned long tasks : BITS_PER_LONG - bits_per(SCHED_CAPACITY_SCALE);
> };
>
> defined as part of:
>
> struct uclamp_rq {
> unsigned int value;
> struct uclamp_bucket bucket[UCLAMP_BUCKETS];
> };
>
>
> So, it's an array of UCLAMP_BUCKETS (value, tasks) pairs.
>
> > uclamp_rq size depends on UCLAMP_BUCKETS configurable to be up
> > to 20. sizeof(long)*20 is already more than 64 bytes. What am I
> > missing?
>
> Right, the comment above refers to the default configuration, which is
> 5 buckets. With that configuration we have:
>
>
> $> pahole kernel/sched/core.o
>
> ---8<---
> struct uclamp_bucket {
> long unsigned int value:11; /* 0:53 8 */
> long unsigned int tasks:53; /* 0: 0 8 */
>
> /* size: 8, cachelines: 1, members: 2 */
> /* last cacheline: 8 bytes */
> };
>
> struct uclamp_rq {
> unsigned int value; /* 0 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct uclamp_bucket bucket[5]; /* 8 40 */
>
> /* size: 48, cachelines: 1, members: 2 */
> /* sum members: 44, holes: 1, sum holes: 4 */
> /* last cacheline: 48 bytes */
> };
>
> struct rq {
> // ...
> /* --- cacheline 2 boundary (128 bytes) --- */
> struct uclamp_rq uclamp[2]; /* 128 96 */
> /* --- cacheline 3 boundary (192 bytes) was 32 bytes ago --- */
> // ...
> };
> ---8<---
>
> Where you see the array fits into a single cache line.
>
> Actually I notice now that, since when we removed the bucket dedicated
> to the default values, we now have some spare space and we can
> probably increase the default (and minimum) value of UCLAMP_BUCKETS to
> be 7.
>
> This will uses two full cache lines in struct rq, one for each clamp
> index... Although 7 it's a bit of a odd number and gives by default
> buckets of ~14% size instead of the ~20%.
>
> Thoughts ?
Got it. From reading the documentation at the beginning my impression
was that whatever value I choose within allowed 5-20 range it would
still fit in a cache line. To disambiguate it might be worse
mentioning that this is true for the default value or for values up to
7. Thanks!
> [...]
>
> --
> #include <best/regards.h>
>
> Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 15:07 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190314133211.GJ5996@hirez.programming.kicks-ass.net>
On 14-Mar 14:32, Peter Zijlstra wrote:
> On Thu, Mar 14, 2019 at 12:13:15PM +0000, Patrick Bellasi wrote:
> > > I'd be most impressed if they pull this off. Check the generated code
> > > and see I suppose :-)
> >
> > On x86 the code generated looks exactly the same:
> >
> > https://godbolt.org/z/PjmA7k
>
> Argh, they do mult by inverse to avoid the division, and can do this
> because its a constant.
>
> And then yes, your arm version looks worse.
your "arm version" is worst then x86, or "your version" is worse?
IOW, should I keep the code as the original? Do you prefer your
version? Or... we don't really care...
> It does what I expected with -Os, but as Rutland said the other day,
> that stands for Optimize for Sadness.
Yes, I guess we cannot optimize for all flags... however, just let me
know what you prefer and I'll put that version in ;)
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 15:00 UTC (permalink / raw)
To: Dietmar Eggemann
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Peter Zijlstra,
Tejun Heo, Rafael J . Wysocki, Vincent Guittot, Viresh Kumar,
Paul Turner, Quentin Perret, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190313151535.q5ivsuywvwkewrk5@e110439-lin>
On 13-Mar 15:15, Patrick Bellasi wrote:
> On 12-Mar 13:52, Dietmar Eggemann wrote:
> > On 2/8/19 11:05 AM, Patrick Bellasi wrote:
[...]
> > > + * within each bucket the exact "requested" clamp value whenever all tasks
> > > + * RUNNABLE in that bucket require the same clamp.
> > > + */
> > > +static inline void uclamp_rq_inc_id(struct task_struct *p, struct rq *rq,
> > > + unsigned int clamp_id)
> > > +{
> > > + unsigned int bucket_id = p->uclamp[clamp_id].bucket_id;
> > > + unsigned int rq_clamp, bkt_clamp, tsk_clamp;
> >
> > Wouldn't it be easier to have a pointer to the task's and rq's uclamp
> > structure as well to the bucket?
> >
> > - unsigned int bucket_id = p->uclamp[clamp_id].bucket_id;
> > + struct uclamp_se *uc_se = &p->uclamp[clamp_id];
> > + struct uclamp_rq *uc_rq = &rq->uclamp[clamp_id];
> > + struct uclamp_bucket *bucket = &uc_rq->bucket[uc_se->bucket_id];
>
> I think I went back/forth a couple of times in using pointer or the
> extended version, which both have pros and cons.
>
> I personally prefer the pointers as you suggest but I've got the
> impression in the past that since everybody cleared "basic C trainings"
> it's not so difficult to read the code above too.
>
> > The code in uclamp_rq_inc_id() and uclamp_rq_dec_id() for example becomes
> > much more readable.
>
> Agree... let's try to switch once again in v8 and see ;)
This is not as straightforward as I thought.
We either still need local variables to use with max(), which does not
play well with bitfields values, or we have to avoid using it and go
back to conditional updates:
---8<---
static inline void uclamp_rq_inc_id(struct task_struct *p, struct rq *rq,
unsigned int clamp_id)
{
struct uclamp_rq *uc_rq = &rq->uclamp[clamp_id];
struct uclamp_req *uc_se = &p->uclamp_se[clamp_id];
struct uclamp_bucket *bucket = &uc_rq->bucket[uc_se->bucket_id];
bucket->tasks++;
/*
* Local clamping: rq's buckets always track the max "requested"
* clamp value from all RUNNABLE tasks in that bucket.
*/
if (uc_se->value > bucket->value)
bucket->value = uc_se->value;
if (uc_se->value > READ_ONCE(uc_rq->value))
WRITE_ONCE(uc_rq->value, uc_se->value);
}
---8<---
I remember Peter asking for max() in one of the past reviews.. but the
code above looks simpler to me too... let see if this time it can be
accepted. :)
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 14:54 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Dietmar Eggemann, LKML, linux-pm, linux-api, Ingo Molnar,
Peter Zijlstra, Tejun Heo, Rafael J . Wysocki, Vincent Guittot,
Viresh Kumar, Paul Turner, Quentin Perret, Morten Rasmussen,
Juri Lelli, Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <CAJuCfpGzxnV8UH4h0O7V03RrecEkJA3JQ3zJTu=b8SHhnOo_mg@mail.gmail.com>
On 13-Mar 14:01, Suren Baghdasaryan wrote:
> On Wed, Mar 13, 2019 at 8:15 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
> >
> > On 12-Mar 13:52, Dietmar Eggemann wrote:
> > > On 2/8/19 11:05 AM, Patrick Bellasi wrote:
[...]
> > > > +static inline unsigned int uclamp_bucket_value(unsigned int clamp_value)
> > > > +{
> > > > + return UCLAMP_BUCKET_DELTA * uclamp_bucket_id(clamp_value);
> > > > +}
> > >
> > > Soemthing like uclamp_bucket_nominal_value() should be clearer.
> >
> > Maybe... can update it in v8
> >
>
> uclamp_bucket_base_value is a little shorter, just to consider :)
Right, I also like it better ;)
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 14:46 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: LKML, linux-pm, linux-api, Ingo Molnar, Peter Zijlstra, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <CAJuCfpGAZ6XxJj=e-wVzGx3zmRRLPs=wXm8K0BcjWOm5hM0FcQ@mail.gmail.com>
On 13-Mar 14:32, Suren Baghdasaryan wrote:
> On Fri, Feb 8, 2019 at 2:06 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
> >
> > Utilization clamping allows to clamp the CPU's utilization within a
> > [util_min, util_max] range, depending on the set of RUNNABLE tasks on
> > that CPU. Each task references two "clamp buckets" defining its minimum
> > and maximum (util_{min,max}) utilization "clamp values". A CPU's clamp
> > bucket is active if there is at least one RUNNABLE tasks enqueued on
> > that CPU and refcounting that bucket.
> >
> > When a task is {en,de}queued {on,from} a rq, the set of active clamp
> > buckets on that CPU can change. Since each clamp bucket enforces a
> > different utilization clamp value, when the set of active clamp buckets
> > changes, a new "aggregated" clamp value is computed for that CPU.
> >
> > Clamp values are always MAX aggregated for both util_min and util_max.
> > This ensures that no tasks can affect the performance of other
> > co-scheduled tasks which are more boosted (i.e. with higher util_min
> > clamp) or less capped (i.e. with higher util_max clamp).
> >
> > Each task has a:
> > task_struct::uclamp[clamp_id]::bucket_id
> > to track the "bucket index" of the CPU's clamp bucket it refcounts while
> > enqueued, for each clamp index (clamp_id).
> >
> > Each CPU's rq has a:
> > rq::uclamp[clamp_id]::bucket[bucket_id].tasks
> > to track how many tasks, currently RUNNABLE on that CPU, refcount each
> > clamp bucket (bucket_id) of a clamp index (clamp_id).
> >
> > Each CPU's rq has also a:
> > rq::uclamp[clamp_id]::bucket[bucket_id].value
> > to track the clamp value of each clamp bucket (bucket_id) of a clamp
> > index (clamp_id).
> >
> > The rq::uclamp::bucket[clamp_id][] array is scanned every time we need
> > to find a new MAX aggregated clamp value for a clamp_id. This operation
> > is required only when we dequeue the last task of a clamp bucket
> > tracking the current MAX aggregated clamp value. In these cases, the CPU
> > is either entering IDLE or going to schedule a less boosted or more
> > clamped task.
> > The expected number of different clamp values, configured at build time,
> > is small enough to fit the full unordered array into a single cache
> > line.
>
> I assume you are talking about "struct uclamp_rq uclamp[UCLAMP_CNT]"
> here.
No, I'm talking about the rq::uclamp::bucket[clamp_id][], which is an
array of:
struct uclamp_bucket {
unsigned long value : bits_per(SCHED_CAPACITY_SCALE);
unsigned long tasks : BITS_PER_LONG - bits_per(SCHED_CAPACITY_SCALE);
};
defined as part of:
struct uclamp_rq {
unsigned int value;
struct uclamp_bucket bucket[UCLAMP_BUCKETS];
};
So, it's an array of UCLAMP_BUCKETS (value, tasks) pairs.
> uclamp_rq size depends on UCLAMP_BUCKETS configurable to be up
> to 20. sizeof(long)*20 is already more than 64 bytes. What am I
> missing?
Right, the comment above refers to the default configuration, which is
5 buckets. With that configuration we have:
$> pahole kernel/sched/core.o
---8<---
struct uclamp_bucket {
long unsigned int value:11; /* 0:53 8 */
long unsigned int tasks:53; /* 0: 0 8 */
/* size: 8, cachelines: 1, members: 2 */
/* last cacheline: 8 bytes */
};
struct uclamp_rq {
unsigned int value; /* 0 4 */
/* XXX 4 bytes hole, try to pack */
struct uclamp_bucket bucket[5]; /* 8 40 */
/* size: 48, cachelines: 1, members: 2 */
/* sum members: 44, holes: 1, sum holes: 4 */
/* last cacheline: 48 bytes */
};
struct rq {
// ...
/* --- cacheline 2 boundary (128 bytes) --- */
struct uclamp_rq uclamp[2]; /* 128 96 */
/* --- cacheline 3 boundary (192 bytes) was 32 bytes ago --- */
// ...
};
---8<---
Where you see the array fits into a single cache line.
Actually I notice now that, since when we removed the bucket dedicated
to the default values, we now have some spare space and we can
probably increase the default (and minimum) value of UCLAMP_BUCKETS to
be 7.
This will uses two full cache lines in struct rq, one for each clamp
index... Although 7 it's a bit of a odd number and gives by default
buckets of ~14% size instead of the ~20%.
Thoughts ?
[...]
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Peter Zijlstra @ 2019-03-14 13:32 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190314121315.juqpsqu5cwouuqpp@e110439-lin>
On Thu, Mar 14, 2019 at 12:13:15PM +0000, Patrick Bellasi wrote:
> > I'd be most impressed if they pull this off. Check the generated code
> > and see I suppose :-)
>
> On x86 the code generated looks exactly the same:
>
> https://godbolt.org/z/PjmA7k
Argh, they do mult by inverse to avoid the division, and can do this
because its a constant.
And then yes, your arm version looks worse.
It does what I expected with -Os, but as Rutland said the other day,
that stands for Optimize for Sadness.
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Peter Zijlstra @ 2019-03-14 13:27 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190314110330.rshrdeoxgzt6pk5a@e110439-lin>
On Thu, Mar 14, 2019 at 11:03:30AM +0000, Patrick Bellasi wrote:
> void uclamp_rq_dec_id(struct rq *rq, int clamp_id, int bucket_id) {
> if (__builtin_expect(!!(rq->uclamp[clamp_id].bucket[bucket_id].tasks), 1))
> return;
> rq->uclamp[clamp_id].bucket[bucket_id].tasks--;
> }
> ---8<---
>
> generates something like:
>
> ---8<---
> uclamp_rq_dec_id:
> sxtw x1, w1
> add x3, x1, x1, lsl 1
> lsl x3, x3, 2
> sub x3, x3, x1
> lsl x3, x3, 2
> add x2, x3, x2, sxtw 3
> add x0, x0, x2
> ldr w1, [x0, 8]
> cbz w1, .L4
> ret
> .L4:
> mov w1, -1
> str w1, [x0, 8]
> ret
> ---8<---
>
>
> which looks "sane" and quite expected, isn't it?
Yep, thanks! Sometimes I worry about silly things.
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 12:43 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Peter Zijlstra, LKML, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <CAJuCfpHNgSOR_ZujZ4XFS2sqGUaJKPOYoc8aqToO=qxA2LhBag@mail.gmail.com>
On 13-Mar 14:23, Suren Baghdasaryan wrote:
> On Wed, Mar 13, 2019 at 6:52 AM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Fri, Feb 08, 2019 at 10:05:40AM +0000, Patrick Bellasi wrote:
> > > +/*
> > > + * When a task is enqueued on a rq, the clamp bucket currently defined by the
> > > + * task's uclamp::bucket_id is reference counted on that rq. This also
> > > + * immediately updates the rq's clamp value if required.
> > > + *
> > > + * Since tasks know their specific value requested from user-space, we track
> > > + * within each bucket the maximum value for tasks refcounted in that bucket.
> > > + * This provide a further aggregation (local clamping) which allows to track
> > > + * within each bucket the exact "requested" clamp value whenever all tasks
> > > + * RUNNABLE in that bucket require the same clamp.
> > > + */
> > > +static inline void uclamp_rq_inc_id(struct task_struct *p, struct rq *rq,
> > > + unsigned int clamp_id)
> > > +{
> > > + unsigned int bucket_id = p->uclamp[clamp_id].bucket_id;
> > > + unsigned int rq_clamp, bkt_clamp, tsk_clamp;
> > > +
> > > + rq->uclamp[clamp_id].bucket[bucket_id].tasks++;
> > > +
> > > + /*
> > > + * Local clamping: rq's buckets always track the max "requested"
> > > + * clamp value from all RUNNABLE tasks in that bucket.
> > > + */
> > > + tsk_clamp = p->uclamp[clamp_id].value;
> > > + bkt_clamp = rq->uclamp[clamp_id].bucket[bucket_id].value;
> > > + rq->uclamp[clamp_id].bucket[bucket_id].value = max(bkt_clamp, tsk_clamp);
> >
> > So, if I read this correct:
> >
> > - here we track a max value in a bucket,
> >
> > > + rq_clamp = READ_ONCE(rq->uclamp[clamp_id].value);
> > > + WRITE_ONCE(rq->uclamp[clamp_id].value, max(rq_clamp, tsk_clamp));
> > > +}
> > > +
> > > +/*
> > > + * When a task is dequeued from a rq, the clamp bucket reference counted by
> > > + * the task is released. If this is the last task reference counting the rq's
> > > + * max active clamp value, then the rq's clamp value is updated.
> > > + * Both the tasks reference counter and the rq's cached clamp values are
> > > + * expected to be always valid, if we detect they are not we skip the updates,
> > > + * enforce a consistent state and warn.
> > > + */
> > > +static inline void uclamp_rq_dec_id(struct task_struct *p, struct rq *rq,
> > > + unsigned int clamp_id)
> > > +{
> > > + unsigned int bucket_id = p->uclamp[clamp_id].bucket_id;
> > > + unsigned int rq_clamp, bkt_clamp;
> > > +
> > > + SCHED_WARN_ON(!rq->uclamp[clamp_id].bucket[bucket_id].tasks);
> > > + if (likely(rq->uclamp[clamp_id].bucket[bucket_id].tasks))
> > > + rq->uclamp[clamp_id].bucket[bucket_id].tasks--;
> > > +
> > > + /*
> > > + * Keep "local clamping" simple and accept to (possibly) overboost
> > > + * still RUNNABLE tasks in the same bucket.
> > > + */
> > > + if (likely(rq->uclamp[clamp_id].bucket[bucket_id].tasks))
> > > + return;
> >
> > (Oh man, I hope that generates semi sane code; long live CSE passes I
> > suppose)
> >
> > But we never decrement that bkt_clamp value on dequeue.
> >
> > > + bkt_clamp = rq->uclamp[clamp_id].bucket[bucket_id].value;
> > > +
> > > + /* The rq's clamp value is expected to always track the max */
> > > + rq_clamp = READ_ONCE(rq->uclamp[clamp_id].value);
> > > + SCHED_WARN_ON(bkt_clamp > rq_clamp);
> > > + if (bkt_clamp >= rq_clamp) {
> >
> > head hurts, this reads ==, how can this ever not be so?
> >
> > > + /*
> > > + * Reset rq's clamp bucket value to its nominal value whenever
> > > + * there are anymore RUNNABLE tasks refcounting it.
> >
> > -ENOPARSE
> >
> > > + */
> > > + rq->uclamp[clamp_id].bucket[bucket_id].value =
> > > + uclamp_bucket_value(rq_clamp);
> >
> > But basically you decrement the bucket value to the nominal value.
> >
> > > + uclamp_rq_update(rq, clamp_id);
> > > + }
> > > +}
> >
> > Given all that, what is to stop the bucket value to climbing to
> > uclamp_bucket_value(+1)-1 and staying there (provided there's someone
> > runnable)?
> >
> > Why are we doing this... ?
>
> I agree with Peter, this part of the patch was the hardest to read.
> SCHED_WARN_ON line makes sense to me. The condition that follows and
> the following comment are a little baffling. Condition seems to
> indicate that the code that follows should be executed only if we are
> in the top-most occupied bucket (the bucket which has tasks and has
> the highest uclamp value).
> So this bucket just lost its last task and we should update
> rq->uclamp[clamp_id].value.
Right.
> However that's not exactly what the code does... It also resets
> rq->uclamp[clamp_id].bucket[bucket_id].value.
Right...
> So if I understand correctly, unless the bucket that just lost its
> last task is the top-most one its value will not be reset to nominal
> value. That looks like a bug to me. Am I missing something?
... and I think you've got a point here!
The reset to nominal value line should be done unconditionally.
I'll move it outside its current block. Thanks for spotting it.
> Side note: some more explanation would be very helpful.
Will move that "bucket local max" management code into a separate
patch as suggested by Peter. Hopefully that should make the logic more
clear and allows me to add some notes in the changelog.
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 12:22 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: Peter Zijlstra, LKML, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <CAJuCfpHy0+6f9mRd+cF-oQrrUWJ4m8mOQJ7XMzkDL4mhfYyj2g@mail.gmail.com>
On 13-Mar 14:08, Suren Baghdasaryan wrote:
> On Wed, Mar 13, 2019 at 12:46 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > On Wed, Mar 13, 2019 at 03:23:59PM +0000, Patrick Bellasi wrote:
> > > On 13-Mar 15:09, Peter Zijlstra wrote:
> > > > On Fri, Feb 08, 2019 at 10:05:40AM +0000, Patrick Bellasi wrote:
> >
> > > > > +static inline void uclamp_rq_update(struct rq *rq, unsigned int clamp_id)
> > > > > +{
> > > > > + struct uclamp_bucket *bucket = rq->uclamp[clamp_id].bucket;
> > > > > + unsigned int max_value = uclamp_none(clamp_id);
> > > >
> > > > That's 1024 for uclamp_max
> > > >
> > > > > + unsigned int bucket_id;
> > > > > +
> > > > > + /*
> > > > > + * Both min and max clamps are MAX aggregated, thus the topmost
> > > > > + * bucket with some tasks defines the rq's clamp value.
> > > > > + */
> > > > > + bucket_id = UCLAMP_BUCKETS;
> > > > > + do {
> > > > > + --bucket_id;
> > > > > + if (!rq->uclamp[clamp_id].bucket[bucket_id].tasks)
> > > > > + continue;
> > > > > + max_value = bucket[bucket_id].value;
> > > >
> > > > but this will then _lower_ it. That's not a MAX aggregate.
> > >
> > > For uclamp_max we want max_value=1024 when there are no active tasks,
> > > which means: no max clamp enforced on CFS/RT "idle" cpus.
> > >
> > > If instead there are active RT/CFS tasks then we want the clamp value
> > > of the max group, which means: MAX aggregate active clamps.
> > >
> > > That's what the code above does and the comment says.
> >
> > That's (obviously) not how I read it... maybe something like:
> >
> > static inline void uclamp_rq_update(struct rq *rq, unsigned int clamp_id)
> > {
> > struct uclamp_bucket *bucket = rq->uclamp[clamp_id].bucket;
> > int i;
> >
> > /*
> > * Since both min and max clamps are max aggregated, find the
> > * top most bucket with tasks in.
> > */
> > for (i = UCLMAP_BUCKETS-1; i>=0; i--) {
> > if (!bucket[i].tasks)
> > continue;
> > return bucket[i].value;
> > }
> >
> > /* No tasks -- default clamp values */
> > return uclamp_none(clamp_id);
> > }
> >
> > would make it clearer?
>
> This way it's also more readable/obvious when it's used inside
> uclamp_rq_dec_id, assuming uclamp_rq_update is renamed into smth like
> get_max_rq_uclamp.
Rightm, I have now something like that:
---8<---
static inline unsigned int uclamp_rq_max_value(struct rq *rq, unsigned int clamp_id)
{
struct uclamp_bucket *bucket = rq->uclamp[clamp_id].bucket;
int bucket_id;
/*
* Since both min and max clamps are max aggregated, find the
* top most bucket with tasks in.
*/
for (bucket_id = UCLMAP_BUCKETS-1; bucket_id >= 0; bucket_id--) {
if (!bucket[bucket_id].tasks)
continue;
return bucket[bucket_id].value;
}
/* No tasks -- default clamp value */
return uclamp_none(clamp_id);
}
static inline void uclamp_rq_dec_id(struct task_struct *p, struct rq *rq,
unsigned int clamp_id)
{
//...
if (bucket->value >= rq_clamp) {
/*
* Reset rq's clamp bucket value to its nominal value whenever
* there are anymore RUNNABLE tasks refcounting it.
*/
bucket->value = uclamp_bucket_nominal_value(rq_clamp);
WRITE_ONCE(uc_rq->value, uclamp_rq_max_value(rq, clamp_id));
}
}
---8<---
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 12:13 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190313194838.GS2482@worktop.programming.kicks-ass.net>
On 13-Mar 20:48, Peter Zijlstra wrote:
> On Wed, Mar 13, 2019 at 04:12:29PM +0000, Patrick Bellasi wrote:
> > On 13-Mar 14:40, Peter Zijlstra wrote:
> > > On Fri, Feb 08, 2019 at 10:05:40AM +0000, Patrick Bellasi wrote:
> > > > +static inline unsigned int uclamp_bucket_id(unsigned int clamp_value)
> > > > +{
> > > > + return clamp_value / UCLAMP_BUCKET_DELTA;
> > > > +}
> > > > +
> > > > +static inline unsigned int uclamp_bucket_value(unsigned int clamp_value)
> > > > +{
> > > > + return UCLAMP_BUCKET_DELTA * uclamp_bucket_id(clamp_value);
> > >
> > > return clamp_value - (clamp_value % UCLAMP_BUCKET_DELTA);
> > >
> > > might generate better code; just a single division, instead of a div and
> > > mult.
> >
> > Wondering if compilers cannot do these optimizations... but yes, looks
> > cool and will do it in v8, thanks.
>
> I'd be most impressed if they pull this off. Check the generated code
> and see I suppose :-)
On x86 the code generated looks exactly the same:
https://godbolt.org/z/PjmA7k
While on on arm64 it seems the difference boils down to:
- one single "mul" instruction
vs
- two instructions: "sub" _plus_ one "multiply subtract"
https://godbolt.org/z/0shU0S
So, if I din't get something wrong... perhaps the original version is
even better, isn't it?
Test code:
---8<---
#define UCLAMP_BUCKET_DELTA 52
static inline unsigned int uclamp_bucket_id(unsigned int clamp_value)
{
return clamp_value / UCLAMP_BUCKET_DELTA;
}
static inline unsigned int uclamp_bucket_value1(unsigned int clamp_value)
{
return UCLAMP_BUCKET_DELTA * uclamp_bucket_id(clamp_value);
}
static inline unsigned int uclamp_bucket_value2(unsigned int clamp_value)
{
return clamp_value - (clamp_value % UCLAMP_BUCKET_DELTA);
}
int test1(int argc, char *argv[]) {
return uclamp_bucket_value1(argc);
}
int test2(int argc, char *argv[]) {
return uclamp_bucket_value2(argc);
}
int test3(int argc, char *argv[]) {
return uclamp_bucket_value1(argc) - uclamp_bucket_value2(argc);
}
---8<---
which gives on arm64:
---8<---
test1:
mov w1, 60495
movk w1, 0x4ec4, lsl 16
umull x0, w0, w1
lsr x0, x0, 36
mov w1, 52
mul w0, w0, w1
ret
test2:
mov w1, 60495
movk w1, 0x4ec4, lsl 16
umull x1, w0, w1
lsr x1, x1, 36
mov w2, 52
msub w1, w1, w2, w0
sub w0, w0, w1
ret
test3:
mov w0, 0
ret
---8<---
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 11:45 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190313194619.GR2482@worktop.programming.kicks-ass.net>
On 13-Mar 20:46, Peter Zijlstra wrote:
> On Wed, Mar 13, 2019 at 03:23:59PM +0000, Patrick Bellasi wrote:
> > On 13-Mar 15:09, Peter Zijlstra wrote:
> > > On Fri, Feb 08, 2019 at 10:05:40AM +0000, Patrick Bellasi wrote:
>
> > > > +static inline void uclamp_rq_update(struct rq *rq, unsigned int clamp_id)
> > > > +{
> > > > + struct uclamp_bucket *bucket = rq->uclamp[clamp_id].bucket;
> > > > + unsigned int max_value = uclamp_none(clamp_id);
> > >
> > > That's 1024 for uclamp_max
> > >
> > > > + unsigned int bucket_id;
> > > > +
> > > > + /*
> > > > + * Both min and max clamps are MAX aggregated, thus the topmost
> > > > + * bucket with some tasks defines the rq's clamp value.
> > > > + */
> > > > + bucket_id = UCLAMP_BUCKETS;
> > > > + do {
> > > > + --bucket_id;
> > > > + if (!rq->uclamp[clamp_id].bucket[bucket_id].tasks)
> > > > + continue;
> > > > + max_value = bucket[bucket_id].value;
> > >
> > > but this will then _lower_ it. That's not a MAX aggregate.
> >
> > For uclamp_max we want max_value=1024 when there are no active tasks,
> > which means: no max clamp enforced on CFS/RT "idle" cpus.
> >
> > If instead there are active RT/CFS tasks then we want the clamp value
> > of the max group, which means: MAX aggregate active clamps.
> >
> > That's what the code above does and the comment says.
>
> That's (obviously) not how I read it.... maybe something like:
>
> static inline void uclamp_rq_update(struct rq *rq, unsigned int clamp_id)
> {
> struct uclamp_bucket *bucket = rq->uclamp[clamp_id].bucket;
> int i;
>
> /*
> * Since both min and max clamps are max aggregated, find the
> * top most bucket with tasks in.
> */
> for (i = UCLMAP_BUCKETS-1; i>=0; i--) {
> if (!bucket[i].tasks)
> continue;
> return bucket[i].value;
> }
>
> /* No tasks -- default clamp value */
> return uclamp_none(clamp_id);
> }
>
> would make it clearer?
Fine for me, I'll then change the name in something else since that's
not more an "_update" by moving the WRITE_ONCE into the caller.
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 11:18 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190313193916.GQ2482@worktop.programming.kicks-ass.net>
On 13-Mar 20:39, Peter Zijlstra wrote:
> On Wed, Mar 13, 2019 at 03:59:54PM +0000, Patrick Bellasi wrote:
> > On 13-Mar 14:52, Peter Zijlstra wrote:
>
> > Because of backetization, we potentially end up tracking tasks with
> > different requested clamp values in the same bucket.
> >
> > For example, with 20% bucket size, we can have:
> > Task1: util_min=25%
> > Task2: util_min=35%
> > accounted in the same bucket.
>
> > > Given all that, what is to stop the bucket value to climbing to
> > > uclamp_bucket_value(+1)-1 and staying there (provided there's someone
> > > runnable)?
> >
> > Nothing... but that's an expected consequence of bucketization.
>
> No, it is not.
>
> > > Why are we doing this... ?
> >
> > You can either decide to:
> >
> > a) always boost tasks to just the bucket nominal value
> > thus always penalizing both Task1 and Task2 of the example above
>
> This is the expected behaviour. When was the last time your histogram
> did something like b?
Right, I see what you mean... strictly speaking histograms always do a
floor aggregation.
> > b) always boost tasks to the bucket "max" value
> > thus always overboosting both Task1 and Task2 of the example above
> >
> > The solution above instead has a very good property: in systems
> > where you have only few and well defined clamp values we always
> > provide the exact boost.
> >
> > For example, if your system requires only 23% and 47% boost values
> > (totally random numbers), then you can always get the exact boost
> > required using just 3 bucksts or ~33% size each.
> >
> > In systems where you don't know which boost values you will have, you
> > can still defined the maximum overboost granularity you accept for
> > each task by just tuning the number of clamp groups. For example, with
> > 20 groups you can have a 5% max overboost.
>
> Maybe, but this is not a direct concequence of buckets, but an
> additional heuristic that might work well in this case.
Right... that's the point.
We started with mapping to be able to track exact clamp values.
Then we switched to linear mapping to remove the complexity of
mapping, but we would like to still have the possibility to track
exact numbers whenever possible.
> Maybe split this out in a separate patch? So start with the trivial
> bucket, and then do this change on top with the above few paragraphs as
> changelog?
That's doable, otherwise maybe we can just add the above paragraphs to
the changelog of this patch. But give your comment above I assume you
prefer to split it out... just let me know otherwise.
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 01/15] sched/core: uclamp: Add CPU's clamp buckets refcounting
From: Patrick Bellasi @ 2019-03-14 11:03 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan
In-Reply-To: <20190313193056.GP2482@worktop.programming.kicks-ass.net>
On 13-Mar 20:30, Peter Zijlstra wrote:
> On Wed, Mar 13, 2019 at 03:59:54PM +0000, Patrick Bellasi wrote:
> > On 13-Mar 14:52, Peter Zijlstra wrote:
> > > > +static inline void uclamp_rq_dec_id(struct task_struct *p, struct rq *rq,
> > > > + unsigned int clamp_id)
> > > > +{
> > > > + unsigned int bucket_id = p->uclamp[clamp_id].bucket_id;
> > > > + unsigned int rq_clamp, bkt_clamp;
> > > > +
> > > > + SCHED_WARN_ON(!rq->uclamp[clamp_id].bucket[bucket_id].tasks);
> > > > + if (likely(rq->uclamp[clamp_id].bucket[bucket_id].tasks))
> > > > + rq->uclamp[clamp_id].bucket[bucket_id].tasks--;
> > > > +
> > > > + /*
> > > > + * Keep "local clamping" simple and accept to (possibly) overboost
> > > > + * still RUNNABLE tasks in the same bucket.
> > > > + */
> > > > + if (likely(rq->uclamp[clamp_id].bucket[bucket_id].tasks))
> > > > + return;
> > >
> > > (Oh man, I hope that generates semi sane code; long live CSE passes I
> > > suppose)
> >
> > What do you mean ?
>
> that does: 'rq->uclamp[clamp_id].bucket[bucket_id].tasks' three times in
> a row. And yes the compiler _should_ dtrt, but....
Sorry, don't follow you here... but it's an interesting point. :)
The code above becomes:
if (__builtin_expect(!!(rq->uclamp[clamp_id].bucket[bucket_id].tasks), 1))
return;
Are you referring to the resolution of the memory references, i.e
1) rq->uclamp
2) rq->uclamp[clamp_id]
3) rq->uclamp[clamp_id].bucket[bucket_id]
?
By playing with:
https://godbolt.org/z/OPLpyR
I can see that this simplified version:
---8<---
#define BUCKETS 5
#define CLAMPS 2
struct uclamp {
unsigned int value;
struct bucket {
unsigned int value;
unsigned int tasks;
} bucket[BUCKETS];
};
struct rq {
struct uclamp uclamp[CLAMPS];
};
void uclamp_rq_dec_id(struct rq *rq, int clamp_id, int bucket_id) {
if (__builtin_expect(!!(rq->uclamp[clamp_id].bucket[bucket_id].tasks), 1))
return;
rq->uclamp[clamp_id].bucket[bucket_id].tasks--;
}
---8<---
generates something like:
---8<---
uclamp_rq_dec_id:
sxtw x1, w1
add x3, x1, x1, lsl 1
lsl x3, x3, 2
sub x3, x3, x1
lsl x3, x3, 2
add x2, x3, x2, sxtw 3
add x0, x0, x2
ldr w1, [x0, 8]
cbz w1, .L4
ret
.L4:
mov w1, -1
str w1, [x0, 8]
ret
---8<---
which looks "sane" and quite expected, isn't it?
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v7 02/15] sched/core: uclamp: Enforce last task UCLAMP_MAX
From: Suren Baghdasaryan @ 2019-03-14 0:29 UTC (permalink / raw)
To: Patrick Bellasi
Cc: Peter Zijlstra, LKML, linux-pm, linux-api, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle
In-Reply-To: <20190313161646.fp2gswuqgzi7z7ow@e110439-lin>
On Wed, Mar 13, 2019 at 9:16 AM Patrick Bellasi <patrick.bellasi@arm.com> wrote:
>
> On 13-Mar 15:12, Peter Zijlstra wrote:
> > On Fri, Feb 08, 2019 at 10:05:41AM +0000, Patrick Bellasi wrote:
> > > +static inline void uclamp_idle_reset(struct rq *rq, unsigned int clamp_id,
> > > + unsigned int clamp_value)
> > > +{
> > > + /* Reset max-clamp retention only on idle exit */
> > > + if (!(rq->uclamp_flags & UCLAMP_FLAG_IDLE))
> > > + return;
> > > +
> > > + WRITE_ONCE(rq->uclamp[clamp_id].value, clamp_value);
> > > +
> > > + /*
> > > + * This function is called for both UCLAMP_MIN (before) and UCLAMP_MAX
> > > + * (after). The idle flag is reset only the second time, when we know
> > > + * that UCLAMP_MIN has been already updated.
> >
> > Why do we care? That is, what is this comment trying to tell us.
>
> Right, the code is clear enough, I'll remove this comment.
It would be probably even clearer if rq->uclamp_flags &=
~UCLAMP_FLAG_IDLE is done from inside uclamp_rq_inc after
uclamp_rq_inc_id for both clamps is called.
> >
> > > + */
> > > + if (clamp_id == UCLAMP_MAX)
> > > + rq->uclamp_flags &= ~UCLAMP_FLAG_IDLE;
> > > +}
>
> --
> #include <best/regards.h>
>
> Patrick Bellasi
^ permalink raw reply
* Re: [PATCHv8 07/10] acpi/hmat: Register processor domain to its memory
From: Rafael J. Wysocki @ 2019-03-13 23:22 UTC (permalink / raw)
To: Keith Busch
Cc: Linux Kernel Mailing List, ACPI Devel Maling List,
Linux Memory Management List, Linux API, Greg Kroah-Hartman,
Rafael Wysocki, Dave Hansen, Dan Williams, Jonathan Cameron,
Brice Goglin
In-Reply-To: <20190311205606.11228-8-keith.busch@intel.com>
On Mon, Mar 11, 2019 at 9:55 PM Keith Busch <keith.busch@intel.com> wrote:
>
> If the HMAT Subsystem Address Range provides a valid processor proximity
> domain for a memory domain, or a processor domain matches the performance
> access of the valid processor proximity domain, register the memory
> target with that initiator so this relationship will be visible under
> the node's sysfs directory.
>
> Since HMAT requires valid address ranges have an equivalent SRAT entry,
> verify each memory target satisfies this requirement.
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Keith Busch <keith.busch@intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/acpi/hmat/Kconfig | 3 +-
> drivers/acpi/hmat/hmat.c | 392 +++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 393 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/hmat/Kconfig b/drivers/acpi/hmat/Kconfig
> index 2f7111b7af62..13cddd612a52 100644
> --- a/drivers/acpi/hmat/Kconfig
> +++ b/drivers/acpi/hmat/Kconfig
> @@ -4,4 +4,5 @@ config ACPI_HMAT
> depends on ACPI_NUMA
> help
> If set, this option has the kernel parse and report the
> - platform's ACPI HMAT (Heterogeneous Memory Attributes Table).
> + platform's ACPI HMAT (Heterogeneous Memory Attributes Table),
> + and register memory initiators with their targets.
> diff --git a/drivers/acpi/hmat/hmat.c b/drivers/acpi/hmat/hmat.c
> index 4758beb3b2c1..01a6eddac6f7 100644
> --- a/drivers/acpi/hmat/hmat.c
> +++ b/drivers/acpi/hmat/hmat.c
> @@ -13,11 +13,105 @@
> #include <linux/device.h>
> #include <linux/init.h>
> #include <linux/list.h>
> +#include <linux/list_sort.h>
> #include <linux/node.h>
> #include <linux/sysfs.h>
>
> static __initdata u8 hmat_revision;
>
> +static __initdata LIST_HEAD(targets);
> +static __initdata LIST_HEAD(initiators);
> +static __initdata LIST_HEAD(localities);
> +
> +/*
> + * The defined enum order is used to prioritize attributes to break ties when
> + * selecting the best performing node.
> + */
> +enum locality_types {
> + WRITE_LATENCY,
> + READ_LATENCY,
> + WRITE_BANDWIDTH,
> + READ_BANDWIDTH,
> +};
> +
> +static struct memory_locality *localities_types[4];
> +
> +struct memory_target {
> + struct list_head node;
> + unsigned int memory_pxm;
> + unsigned int processor_pxm;
> + struct node_hmem_attrs hmem_attrs;
> +};
> +
> +struct memory_initiator {
> + struct list_head node;
> + unsigned int processor_pxm;
> +};
> +
> +struct memory_locality {
> + struct list_head node;
> + struct acpi_hmat_locality *hmat_loc;
> +};
> +
> +static __init struct memory_initiator *find_mem_initiator(unsigned int cpu_pxm)
> +{
> + struct memory_initiator *initiator;
> +
> + list_for_each_entry(initiator, &initiators, node)
> + if (initiator->processor_pxm == cpu_pxm)
> + return initiator;
> + return NULL;
> +}
> +
> +static __init struct memory_target *find_mem_target(unsigned int mem_pxm)
> +{
> + struct memory_target *target;
> +
> + list_for_each_entry(target, &targets, node)
> + if (target->memory_pxm == mem_pxm)
> + return target;
> + return NULL;
> +}
> +
> +static __init void alloc_memory_initiator(unsigned int cpu_pxm)
> +{
> + struct memory_initiator *initiator;
> +
> + if (pxm_to_node(cpu_pxm) == NUMA_NO_NODE)
> + return;
> +
> + initiator = find_mem_initiator(cpu_pxm);
> + if (initiator)
> + return;
> +
> + initiator = kzalloc(sizeof(*initiator), GFP_KERNEL);
> + if (!initiator)
> + return;
> +
> + initiator->processor_pxm = cpu_pxm;
> + list_add_tail(&initiator->node, &initiators);
> +}
> +
> +static __init void alloc_memory_target(unsigned int mem_pxm)
> +{
> + struct memory_target *target;
> +
> + if (pxm_to_node(mem_pxm) == NUMA_NO_NODE)
> + return;
> +
> + target = find_mem_target(mem_pxm);
> + if (target)
> + return;
> +
> + target = kzalloc(sizeof(*target), GFP_KERNEL);
> + if (!target)
> + return;
> +
> + target->memory_pxm = mem_pxm;
> + target->processor_pxm = PXM_INVAL;
> + list_add_tail(&target->node, &targets);
> +}
> +
> static __init const char *hmat_data_type(u8 type)
> {
> switch (type) {
> @@ -89,14 +183,83 @@ static __init u32 hmat_normalize(u16 entry, u64 base, u8 type)
> return value;
> }
>
> +static __init void hmat_update_target_access(struct memory_target *target,
> + u8 type, u32 value)
> +{
> + switch (type) {
> + case ACPI_HMAT_ACCESS_LATENCY:
> + target->hmem_attrs.read_latency = value;
> + target->hmem_attrs.write_latency = value;
> + break;
> + case ACPI_HMAT_READ_LATENCY:
> + target->hmem_attrs.read_latency = value;
> + break;
> + case ACPI_HMAT_WRITE_LATENCY:
> + target->hmem_attrs.write_latency = value;
> + break;
> + case ACPI_HMAT_ACCESS_BANDWIDTH:
> + target->hmem_attrs.read_bandwidth = value;
> + target->hmem_attrs.write_bandwidth = value;
> + break;
> + case ACPI_HMAT_READ_BANDWIDTH:
> + target->hmem_attrs.read_bandwidth = value;
> + break;
> + case ACPI_HMAT_WRITE_BANDWIDTH:
> + target->hmem_attrs.write_bandwidth = value;
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static __init void hmat_add_locality(struct acpi_hmat_locality *hmat_loc)
> +{
> + struct memory_locality *loc;
> +
> + loc = kzalloc(sizeof(*loc), GFP_KERNEL);
> + if (!loc) {
> + pr_notice_once("Failed to allocate HMAT locality\n");
> + return;
> + }
> +
> + loc->hmat_loc = hmat_loc;
> + list_add_tail(&loc->node, &localities);
> +
> + switch (hmat_loc->data_type) {
> + case ACPI_HMAT_ACCESS_LATENCY:
> + localities_types[READ_LATENCY] = loc;
> + localities_types[WRITE_LATENCY] = loc;
> + break;
> + case ACPI_HMAT_READ_LATENCY:
> + localities_types[READ_LATENCY] = loc;
> + break;
> + case ACPI_HMAT_WRITE_LATENCY:
> + localities_types[WRITE_LATENCY] = loc;
> + break;
> + case ACPI_HMAT_ACCESS_BANDWIDTH:
> + localities_types[READ_BANDWIDTH] = loc;
> + localities_types[WRITE_BANDWIDTH] = loc;
> + break;
> + case ACPI_HMAT_READ_BANDWIDTH:
> + localities_types[READ_BANDWIDTH] = loc;
> + break;
> + case ACPI_HMAT_WRITE_BANDWIDTH:
> + localities_types[WRITE_BANDWIDTH] = loc;
> + break;
> + default:
> + break;
> + }
> +}
> +
> static __init int hmat_parse_locality(union acpi_subtable_headers *header,
> const unsigned long end)
> {
> struct acpi_hmat_locality *hmat_loc = (void *)header;
> + struct memory_target *target;
> unsigned int init, targ, total_size, ipds, tpds;
> u32 *inits, *targs, value;
> u16 *entries;
> - u8 type;
> + u8 type, mem_hier;
>
> if (hmat_loc->header.length < sizeof(*hmat_loc)) {
> pr_notice("HMAT: Unexpected locality header length: %d\n",
> @@ -105,6 +268,7 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header,
> }
>
> type = hmat_loc->data_type;
> + mem_hier = hmat_loc->flags & ACPI_HMAT_MEMORY_HIERARCHY;
> ipds = hmat_loc->number_of_initiator_Pds;
> tpds = hmat_loc->number_of_target_Pds;
> total_size = sizeof(*hmat_loc) + sizeof(*entries) * ipds * tpds +
> @@ -123,6 +287,7 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header,
> targs = inits + ipds;
> entries = (u16 *)(targs + tpds);
> for (init = 0; init < ipds; init++) {
> + alloc_memory_initiator(inits[init]);
> for (targ = 0; targ < tpds; targ++) {
> value = hmat_normalize(entries[init * tpds + targ],
> hmat_loc->entry_base_unit,
> @@ -130,9 +295,18 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header,
> pr_info(" Initiator-Target[%d-%d]:%d%s\n",
> inits[init], targs[targ], value,
> hmat_data_type_suffix(type));
> +
> + if (mem_hier == ACPI_HMAT_MEMORY) {
> + target = find_mem_target(targs[targ]);
> + if (target && target->processor_pxm == inits[init])
> + hmat_update_target_access(target, type, value);
> + }
> }
> }
>
> + if (mem_hier == ACPI_HMAT_MEMORY)
> + hmat_add_locality(hmat_loc);
> +
> return 0;
> }
>
> @@ -160,6 +334,7 @@ static int __init hmat_parse_proximity_domain(union acpi_subtable_headers *heade
> const unsigned long end)
> {
> struct acpi_hmat_proximity_domain *p = (void *)header;
> + struct memory_target *target;
>
> if (p->header.length != sizeof(*p)) {
> pr_notice("HMAT: Unexpected address range header length: %d\n",
> @@ -175,6 +350,23 @@ static int __init hmat_parse_proximity_domain(union acpi_subtable_headers *heade
> pr_info("HMAT: Memory Flags:%04x Processor Domain:%d Memory Domain:%d\n",
> p->flags, p->processor_PD, p->memory_PD);
>
> + if (p->flags & ACPI_HMAT_MEMORY_PD_VALID) {
> + target = find_mem_target(p->memory_PD);
> + if (!target) {
> + pr_debug("HMAT: Memory Domain missing from SRAT\n");
> + return -EINVAL;
> + }
> + }
> + if (target && p->flags & ACPI_HMAT_PROCESSOR_PD_VALID) {
> + int p_node = pxm_to_node(p->processor_PD);
> +
> + if (p_node == NUMA_NO_NODE) {
> + pr_debug("HMAT: Invalid Processor Domain\n");
> + return -EINVAL;
> + }
> + target->processor_pxm = p_node;
> + }
> +
> return 0;
> }
>
> @@ -198,6 +390,191 @@ static int __init hmat_parse_subtable(union acpi_subtable_headers *header,
> }
> }
>
> +static __init int srat_parse_mem_affinity(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + struct acpi_srat_mem_affinity *ma = (void *)header;
> +
> + if (!ma)
> + return -EINVAL;
> + if (!(ma->flags & ACPI_SRAT_MEM_ENABLED))
> + return 0;
> + alloc_memory_target(ma->proximity_domain);
> + return 0;
> +}
> +
> +static __init u32 hmat_initiator_perf(struct memory_target *target,
> + struct memory_initiator *initiator,
> + struct acpi_hmat_locality *hmat_loc)
> +{
> + unsigned int ipds, tpds, i, idx = 0, tdx = 0;
> + u32 *inits, *targs;
> + u16 *entries;
> +
> + ipds = hmat_loc->number_of_initiator_Pds;
> + tpds = hmat_loc->number_of_target_Pds;
> + inits = (u32 *)(hmat_loc + 1);
> + targs = inits + ipds;
> + entries = (u16 *)(targs + tpds);
> +
> + for (i = 0; i < ipds; i++) {
> + if (inits[i] == initiator->processor_pxm) {
> + idx = i;
> + break;
> + }
> + }
> +
> + if (i == ipds)
> + return 0;
> +
> + for (i = 0; i < tpds; i++) {
> + if (targs[i] == target->memory_pxm) {
> + tdx = i;
> + break;
> + }
> + }
> + if (i == tpds)
> + return 0;
> +
> + return hmat_normalize(entries[idx * tpds + tdx],
> + hmat_loc->entry_base_unit,
> + hmat_loc->data_type);
> +}
> +
> +static __init bool hmat_update_best(u8 type, u32 value, u32 *best)
> +{
> + bool updated = false;
> +
> + if (!value)
> + return false;
> +
> + switch (type) {
> + case ACPI_HMAT_ACCESS_LATENCY:
> + case ACPI_HMAT_READ_LATENCY:
> + case ACPI_HMAT_WRITE_LATENCY:
> + if (!*best || *best > value) {
> + *best = value;
> + updated = true;
> + }
> + break;
> + case ACPI_HMAT_ACCESS_BANDWIDTH:
> + case ACPI_HMAT_READ_BANDWIDTH:
> + case ACPI_HMAT_WRITE_BANDWIDTH:
> + if (!*best || *best < value) {
> + *best = value;
> + updated = true;
> + }
> + break;
> + }
> +
> + return updated;
> +}
> +
> +static int initiator_cmp(void *priv, struct list_head *a, struct list_head *b)
> +{
> + struct memory_initiator *ia;
> + struct memory_initiator *ib;
> + unsigned long *p_nodes = priv;
> +
> + ia = list_entry(a, struct memory_initiator, node);
> + ib = list_entry(b, struct memory_initiator, node);
> +
> + set_bit(ia->processor_pxm, p_nodes);
> + set_bit(ib->processor_pxm, p_nodes);
> +
> + return ia->processor_pxm - ib->processor_pxm;
> +}
> +
> +static __init void hmat_register_target_initiators(struct memory_target *target)
> +{
> + static DECLARE_BITMAP(p_nodes, MAX_NUMNODES);
> + struct memory_initiator *initiator;
> + unsigned int mem_nid, cpu_nid;
> + struct memory_locality *loc = NULL;
> + u32 best = 0;
> + int i;
> +
> + mem_nid = pxm_to_node(target->memory_pxm);
> + /*
> + * If the Address Range Structure provides a local processor pxm, link
> + * only that one. Otherwise, find the best performance attributes and
> + * register all initiators that match.
> + */
> + if (target->processor_pxm != PXM_INVAL) {
> + cpu_nid = pxm_to_node(target->processor_pxm);
> + register_memory_node_under_compute_node(mem_nid, cpu_nid, 0);
> + return;
> + }
> +
> + if (list_empty(&localities))
> + return;
> +
> + /*
> + * We need the initiator list sorted so we can use bitmap_clear for
> + * previously set initiators when we find a better memory accessor.
> + * We'll also use the sorting to prime the candidate nodes with known
> + * initiators.
> + */
> + bitmap_zero(p_nodes, MAX_NUMNODES);
> + list_sort(p_nodes, &initiators, initiator_cmp);
> + for (i = WRITE_LATENCY; i <= READ_BANDWIDTH; i++) {
> + loc = localities_types[i];
> + if (!loc)
> + continue;
> +
> + best = 0;
> + list_for_each_entry(initiator, &initiators, node) {
> + u32 value;
> +
> + if (!test_bit(initiator->processor_pxm, p_nodes))
> + continue;
> +
> + value = hmat_initiator_perf(target, initiator, loc->hmat_loc);
> + if (hmat_update_best(loc->hmat_loc->data_type, value, &best))
> + bitmap_clear(p_nodes, 0, initiator->processor_pxm);
> + if (value != best)
> + clear_bit(initiator->processor_pxm, p_nodes);
> + }
> + if (best)
> + hmat_update_target_access(target, loc->hmat_loc->data_type, best);
> + }
> +
> + for_each_set_bit(i, p_nodes, MAX_NUMNODES) {
> + cpu_nid = pxm_to_node(i);
> + register_memory_node_under_compute_node(mem_nid, cpu_nid, 0);
> + }
> +}
> +
> +static __init void hmat_register_targets(void)
> +{
> + struct memory_target *target;
> +
> + list_for_each_entry(target, &targets, node)
> + hmat_register_target_initiators(target);
> +}
> +
> +static __init void hmat_free_structures(void)
> +{
> + struct memory_target *target, *tnext;
> + struct memory_locality *loc, *lnext;
> + struct memory_initiator *initiator, *inext;
> +
> + list_for_each_entry_safe(target, tnext, &targets, node) {
> + list_del(&target->node);
> + kfree(target);
> + }
> +
> + list_for_each_entry_safe(initiator, inext, &initiators, node) {
> + list_del(&initiator->node);
> + kfree(initiator);
> + }
> +
> + list_for_each_entry_safe(loc, lnext, &localities, node) {
> + list_del(&loc->node);
> + kfree(loc);
> + }
> +}
> +
> static __init int hmat_init(void)
> {
> struct acpi_table_header *tbl;
> @@ -207,6 +584,17 @@ static __init int hmat_init(void)
> if (srat_disabled())
> return 0;
>
> + status = acpi_get_table(ACPI_SIG_SRAT, 0, &tbl);
> + if (ACPI_FAILURE(status))
> + return 0;
> +
> + if (acpi_table_parse_entries(ACPI_SIG_SRAT,
> + sizeof(struct acpi_table_srat),
> + ACPI_SRAT_TYPE_MEMORY_AFFINITY,
> + srat_parse_mem_affinity, 0) < 0)
> + goto out_put;
> + acpi_put_table(tbl);
> +
> status = acpi_get_table(ACPI_SIG_HMAT, 0, &tbl);
> if (ACPI_FAILURE(status))
> return 0;
> @@ -229,7 +617,9 @@ static __init int hmat_init(void)
> goto out_put;
> }
> }
> + hmat_register_targets();
> out_put:
> + hmat_free_structures();
> acpi_put_table(tbl);
> return 0;
> }
> --
> 2.14.4
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox