* + list_debug-introduce-config_debug_list_minimal.patch added to mm-nonmm-unstable branch
@ 2023-08-02 18:41 Andrew Morton
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2023-08-02 18:41 UTC (permalink / raw)
To: mm-commits, yuzenghui, will, trix, suzuki.poulose, oliver.upton,
ojeda, ndesaulniers, nathan, maz, linux, keescook, james.morse,
glider, dvyukov, catalin.marinas, elver, akpm
The patch titled
Subject: list_debug: introduce CONFIG_DEBUG_LIST_MINIMAL
has been added to the -mm mm-nonmm-unstable branch. Its filename is
list_debug-introduce-config_debug_list_minimal.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/list_debug-introduce-config_debug_list_minimal.patch
This patch will later appear in the mm-nonmm-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Marco Elver <elver@google.com>
Subject: list_debug: introduce CONFIG_DEBUG_LIST_MINIMAL
Date: Wed, 2 Aug 2023 17:06:39 +0200
Numerous production kernel configs (see [1, 2]) are choosing to enable
CONFIG_DEBUG_LIST, which is also being recommended by KSPP for hardened
configs [3]. The feature has never been designed with performance in
mind, yet common list manipulation is happening across hot paths all
over the kernel.
Introduce CONFIG_DEBUG_LIST_MINIMAL, which performs list pointer
checking inline, and only upon list corruption delegates to the
reporting slow path.
To generate optimal machine code with CONFIG_DEBUG_LIST_MINIMAL:
1. Elide checking for pointer values which upon dereference would
result in an immediate access fault -- therefore "minimal" checks.
The trade-off is lower-quality error reports.
2. Use the newly introduced __preserve_most function attribute
(available with Clang, but not yet with GCC) to minimize the code
footprint for calling the reporting slow path. As a result,
function size of callers is reduced by avoiding saving registers
before calling the rarely called reporting slow path.
3. Because the inline checks are a subset of the full set of checks in
___list_*_valid(), always return false if the inline checks failed.
This avoids redundant compare and conditional branch right after
return from the slow path.
As a side-effect of the checks being inline, if the compiler can prove
some condition to always be true, it can completely elide some checks.
Running netperf with CONFIG_DEBUG_LIST_MINIMAL (using a Clang compiler
with "preserve_most") shows throughput improvements, in my case of ~7%
on average (up to 20-30% on some test cases).
Link: https://r.android.com/1266735 [1]
Link: https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/blob/main/config [2]
Link: https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project/Recommended_Settings [3]
Link: https://lkml.kernel.org/r/20230802150712.3583252-3-elver@google.com
Signed-off-by: Marco Elver <elver@google.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Tom Rix <trix@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
arch/arm64/kvm/hyp/nvhe/list_debug.c | 2
include/linux/list.h | 56 ++++++++++++++++++++++---
lib/Kconfig.debug | 15 ++++++
lib/list_debug.c | 2
4 files changed, 69 insertions(+), 6 deletions(-)
--- a/arch/arm64/kvm/hyp/nvhe/list_debug.c~list_debug-introduce-config_debug_list_minimal
+++ a/arch/arm64/kvm/hyp/nvhe/list_debug.c
@@ -26,6 +26,7 @@ static inline __must_check bool nvhe_che
/* The predicates checked here are taken from lib/list_debug.c. */
+__list_valid_slowpath
bool ___list_add_valid(struct list_head *new, struct list_head *prev,
struct list_head *next)
{
@@ -37,6 +38,7 @@ bool ___list_add_valid(struct list_head
return true;
}
+__list_valid_slowpath
bool ___list_del_entry_valid(struct list_head *entry)
{
struct list_head *prev, *next;
--- a/include/linux/list.h~list_debug-introduce-config_debug_list_minimal
+++ a/include/linux/list.h
@@ -39,20 +39,64 @@ static inline void INIT_LIST_HEAD(struct
}
#ifdef CONFIG_DEBUG_LIST
-extern bool ___list_add_valid(struct list_head *new,
- struct list_head *prev,
- struct list_head *next);
+
+#ifdef CONFIG_DEBUG_LIST_MINIMAL
+# define __list_valid_slowpath __cold __preserve_most
+#else
+# define __list_valid_slowpath
+#endif
+
+extern bool __list_valid_slowpath ___list_add_valid(struct list_head *new,
+ struct list_head *prev,
+ struct list_head *next);
static __always_inline bool __list_add_valid(struct list_head *new,
struct list_head *prev,
struct list_head *next)
{
- return ___list_add_valid(new, prev, next);
+ bool ret = true;
+
+ if (IS_ENABLED(CONFIG_DEBUG_LIST_MINIMAL)) {
+ /*
+ * In the minimal config, elide checking if next and prev are
+ * NULL, since the immediate dereference of them below would
+ * result in a fault if NULL.
+ *
+ * With the minimal config we can afford to inline the checks,
+ * which also gives the compiler a chance to elide some of them
+ * completely if they can be proven at compile-time. If one of
+ * the pre-conditions does not hold, the slow-path will show a
+ * report which pre-condition failed.
+ */
+ if (likely(next->prev == prev && prev->next == next && new != prev && new != next))
+ return true;
+ ret = false;
+ }
+
+ ret &= ___list_add_valid(new, prev, next);
+ return ret;
}
-extern bool ___list_del_entry_valid(struct list_head *entry);
+extern bool __list_valid_slowpath ___list_del_entry_valid(struct list_head *entry);
static __always_inline bool __list_del_entry_valid(struct list_head *entry)
{
- return ___list_del_entry_valid(entry);
+ bool ret = true;
+
+ if (IS_ENABLED(CONFIG_DEBUG_LIST_MINIMAL)) {
+ struct list_head *prev = entry->prev;
+ struct list_head *next = entry->next;
+
+ /*
+ * In the minimal config, elide checking if next and prev are
+ * NULL, LIST_POISON1 or LIST_POISON2, since the immediate
+ * dereference of them below would result in a fault.
+ */
+ if (likely(prev->next == entry && next->prev == entry))
+ return true;
+ ret = false;
+ }
+
+ ret &= ___list_del_entry_valid(entry);
+ return ret;
}
#else
static inline bool __list_add_valid(struct list_head *new,
--- a/lib/Kconfig.debug~list_debug-introduce-config_debug_list_minimal
+++ a/lib/Kconfig.debug
@@ -1680,6 +1680,21 @@ config DEBUG_LIST
If unsure, say N.
+config DEBUG_LIST_MINIMAL
+ bool "Minimal linked list debug checks"
+ default !DEBUG_KERNEL
+ depends on DEBUG_LIST
+ help
+ Only perform the minimal set of checks in the linked-list walking
+ routines to catch corruptions that are not guaranteed to result in an
+ immediate access fault.
+
+ This trades lower quality error reports for improved performance: the
+ generated code should be more optimal and provide trade-offs that may
+ better serve safety- and performance- critical environments.
+
+ If unsure, say Y.
+
config DEBUG_PLIST
bool "Debug priority linked list manipulation"
depends on DEBUG_KERNEL
--- a/lib/list_debug.c~list_debug-introduce-config_debug_list_minimal
+++ a/lib/list_debug.c
@@ -17,6 +17,7 @@
* attempt).
*/
+__list_valid_slowpath
bool ___list_add_valid(struct list_head *new, struct list_head *prev,
struct list_head *next)
{
@@ -39,6 +40,7 @@ bool ___list_add_valid(struct list_head
}
EXPORT_SYMBOL(___list_add_valid);
+__list_valid_slowpath
bool ___list_del_entry_valid(struct list_head *entry)
{
struct list_head *prev, *next;
_
Patches currently in -mm which might be from elver@google.com are
compiler-attributes-introduce-the-__preserve_most-function-attribute.patch
list_debug-introduce-inline-wrappers-for-debug-checks.patch
list_debug-introduce-config_debug_list_minimal.patch
^ permalink raw reply [flat|nested] 2+ messages in thread
* + list_debug-introduce-config_debug_list_minimal.patch added to mm-nonmm-unstable branch
@ 2023-08-08 18:35 Andrew Morton
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Morton @ 2023-08-08 18:35 UTC (permalink / raw)
To: mm-commits, yuzenghui, will, trix, suzuki.poulose, samitolvanen,
rostedt, peterz, oliver.upton, ojeda, ndesaulniers, nathan, maz,
mark.rutland, linux, keescook, james.morse, glider, dvyukov,
catalin.marinas, elver, akpm
The patch titled
Subject: list_debug: introduce CONFIG_DEBUG_LIST_MINIMAL
has been added to the -mm mm-nonmm-unstable branch. Its filename is
list_debug-introduce-config_debug_list_minimal.patch
This patch will shortly appear at
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/list_debug-introduce-config_debug_list_minimal.patch
This patch will later appear in the mm-nonmm-unstable branch at
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next via the mm-everything
branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
and is updated there every 2-3 working days
------------------------------------------------------
From: Marco Elver <elver@google.com>
Subject: list_debug: introduce CONFIG_DEBUG_LIST_MINIMAL
Date: Tue, 8 Aug 2023 12:17:27 +0200
Numerous production kernel configs (see [1, 2]) are choosing to enable
CONFIG_DEBUG_LIST, which is also being recommended by KSPP for hardened
configs [3]. The feature has never been designed with performance in
mind, yet common list manipulation is happening across hot paths all
over the kernel.
Introduce CONFIG_DEBUG_LIST_MINIMAL, which performs list pointer
checking inline, and only upon list corruption delegates to the
reporting slow path.
To generate optimal machine code with CONFIG_DEBUG_LIST_MINIMAL:
1. Elide checking for pointer values which upon dereference would
result in an immediate access fault -- therefore "minimal" checks.
The trade-off is lower-quality error reports.
2. Use the newly introduced __preserve_most function attribute
(available with Clang, but not yet with GCC) to minimize the code
footprint for calling the reporting slow path. As a result,
function size of callers is reduced by avoiding saving registers
before calling the rarely called reporting slow path.
Note that all TUs in lib/Makefile already disable function tracing,
including list_debug.c, and __preserve_most's implied notrace has
no effect in this case.
3. Because the inline checks are a subset of the full set of checks in
___list_*_valid(), always return false if the inline checks failed.
This avoids redundant compare and conditional branch right after
return from the slow path.
As a side-effect of the checks being inline, if the compiler can prove
some condition to always be true, it can completely elide some checks.
Running netperf with CONFIG_DEBUG_LIST_MINIMAL (using a Clang compiler
with "preserve_most") shows throughput improvements, in my case of ~7%
on average (up to 20-30% on some test cases).
Link: https://r.android.com/1266735 [1]
Link: https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/blob/main/config [2]
Link: https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project/Recommended_Settings [3]
Link: https://lkml.kernel.org/r/20230808102049.465864-3-elver@google.com
Signed-off-by: Marco Elver <elver@google.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: James Morse <james.morse@arm.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Oliver Upton <oliver.upton@linux.dev>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Tom Rix <trix@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: Zenghui Yu <yuzenghui@huawei.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
arch/arm64/kvm/hyp/nvhe/list_debug.c | 2
include/linux/list.h | 64 ++++++++++++++++++++++---
lib/Kconfig.debug | 15 +++++
lib/list_debug.c | 2
4 files changed, 77 insertions(+), 6 deletions(-)
--- a/arch/arm64/kvm/hyp/nvhe/list_debug.c~list_debug-introduce-config_debug_list_minimal
+++ a/arch/arm64/kvm/hyp/nvhe/list_debug.c
@@ -26,6 +26,7 @@ static inline __must_check bool nvhe_che
/* The predicates checked here are taken from lib/list_debug.c. */
+__list_valid_slowpath
bool __list_add_valid_or_report(struct list_head *new, struct list_head *prev,
struct list_head *next)
{
@@ -37,6 +38,7 @@ bool __list_add_valid_or_report(struct l
return true;
}
+__list_valid_slowpath
bool __list_del_entry_valid_or_report(struct list_head *entry)
{
struct list_head *prev, *next;
--- a/include/linux/list.h~list_debug-introduce-config_debug_list_minimal
+++ a/include/linux/list.h
@@ -39,38 +39,90 @@ static inline void INIT_LIST_HEAD(struct
}
#ifdef CONFIG_DEBUG_LIST
+
+#ifdef CONFIG_DEBUG_LIST_MINIMAL
+# define __list_valid_slowpath __cold __preserve_most
+#else
+# define __list_valid_slowpath
+#endif
+
/*
* Performs the full set of list corruption checks before __list_add().
* On list corruption reports a warning, and returns false.
*/
-extern bool __list_add_valid_or_report(struct list_head *new,
- struct list_head *prev,
- struct list_head *next);
+extern bool __list_valid_slowpath __list_add_valid_or_report(struct list_head *new,
+ struct list_head *prev,
+ struct list_head *next);
/*
* Performs list corruption checks before __list_add(). Returns false if a
* corruption is detected, true otherwise.
+ *
+ * With CONFIG_DEBUG_LIST_MINIMAL set, performs minimal list integrity checking
+ * (that do not result in a fault) inline, and only if a corruption is detected
+ * calls the reporting function __list_add_valid_or_report().
*/
static __always_inline bool __list_add_valid(struct list_head *new,
struct list_head *prev,
struct list_head *next)
{
- return __list_add_valid_or_report(new, prev, next);
+ bool ret = true;
+
+ if (IS_ENABLED(CONFIG_DEBUG_LIST_MINIMAL)) {
+ /*
+ * In the minimal config, elide checking if next and prev are
+ * NULL, since the immediate dereference of them below would
+ * result in a fault if NULL.
+ *
+ * With the minimal config we can afford to inline the checks,
+ * which also gives the compiler a chance to elide some of them
+ * completely if they can be proven at compile-time. If one of
+ * the pre-conditions does not hold, the slow-path will show a
+ * report which pre-condition failed.
+ */
+ if (likely(next->prev == prev && prev->next == next && new != prev && new != next))
+ return true;
+ ret = false;
+ }
+
+ ret &= __list_add_valid_or_report(new, prev, next);
+ return ret;
}
/*
* Performs the full set of list corruption checks before __list_del_entry().
* On list corruption reports a warning, and returns false.
*/
-extern bool __list_del_entry_valid_or_report(struct list_head *entry);
+extern bool __list_valid_slowpath __list_del_entry_valid_or_report(struct list_head *entry);
/*
* Performs list corruption checks before __list_del_entry(). Returns false if a
* corruption is detected, true otherwise.
+ *
+ * With CONFIG_DEBUG_LIST_MINIMAL set, performs minimal list integrity checking
+ * (that do not result in a fault) inline, and only if a corruption is detected
+ * calls the reporting function __list_del_entry_valid_or_report().
*/
static __always_inline bool __list_del_entry_valid(struct list_head *entry)
{
- return __list_del_entry_valid_or_report(entry);
+ bool ret = true;
+
+ if (IS_ENABLED(CONFIG_DEBUG_LIST_MINIMAL)) {
+ struct list_head *prev = entry->prev;
+ struct list_head *next = entry->next;
+
+ /*
+ * In the minimal config, elide checking if next and prev are
+ * NULL, LIST_POISON1 or LIST_POISON2, since the immediate
+ * dereference of them below would result in a fault.
+ */
+ if (likely(prev->next == entry && next->prev == entry))
+ return true;
+ ret = false;
+ }
+
+ ret &= __list_del_entry_valid_or_report(entry);
+ return ret;
}
#else
static inline bool __list_add_valid(struct list_head *new,
--- a/lib/Kconfig.debug~list_debug-introduce-config_debug_list_minimal
+++ a/lib/Kconfig.debug
@@ -1680,6 +1680,21 @@ config DEBUG_LIST
If unsure, say N.
+config DEBUG_LIST_MINIMAL
+ bool "Minimal linked list debug checks"
+ default !DEBUG_KERNEL
+ depends on DEBUG_LIST
+ help
+ Only perform the minimal set of checks in the linked-list walking
+ routines to catch corruptions that are not guaranteed to result in an
+ immediate access fault.
+
+ This trades lower quality error reports for improved performance: the
+ generated code should be more optimal and provide trade-offs that may
+ better serve safety- and performance- critical environments.
+
+ If unsure, say Y.
+
config DEBUG_PLIST
bool "Debug priority linked list manipulation"
depends on DEBUG_KERNEL
--- a/lib/list_debug.c~list_debug-introduce-config_debug_list_minimal
+++ a/lib/list_debug.c
@@ -17,6 +17,7 @@
* attempt).
*/
+__list_valid_slowpath
bool __list_add_valid_or_report(struct list_head *new, struct list_head *prev,
struct list_head *next)
{
@@ -39,6 +40,7 @@ bool __list_add_valid_or_report(struct l
}
EXPORT_SYMBOL(__list_add_valid_or_report);
+__list_valid_slowpath
bool __list_del_entry_valid_or_report(struct list_head *entry)
{
struct list_head *prev, *next;
_
Patches currently in -mm which might be from elver@google.com are
compiler_types-introduce-the-clang-__preserve_most-function-attribute.patch
list_debug-introduce-inline-wrappers-for-debug-checks.patch
list_debug-introduce-config_debug_list_minimal.patch
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-08-08 20:11 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-02 18:41 + list_debug-introduce-config_debug_list_minimal.patch added to mm-nonmm-unstable branch Andrew Morton
-- strict thread matches above, loose matches on Subject: below --
2023-08-08 18:35 Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.