2022-08-11 02:59:36

by Ira Weiny

[permalink] [raw]
Subject: [PATCH 0/3] Print CPU at segfault time

From: Ira Weiny <[email protected]>

Changes from RFC:
Drop patch 1 as I misunderstood the code and it is not needed for this
use case
Combine patch 2 and 5 into a patch 3 which stores the CPU only for page
faults

Rik reported that the knowledge of which CPU's are seeing faults can help in
determining which CPUs are failing in a large data center.[1]

Storing the CPU at exception entry time allows this print to report the CPU
which actually took the exception. This still may not be the CPU which is
failing but it should be closer.

Dave and Boris recognized that the auxiliary pt_regs work I did for the PKS
series could help to store this value and avoid passing the CPU throughout the
fault handler call stack.

The patches I sent for RFC had a lot more overhead than is needed to report the
CPU in a page fault.[2] After the discussions the generic save/restore of the
auxiliary pt_regs is overkill for the current use case. Skip that overhead and
only store the CPU in the page fault path.

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/

Ira Weiny (2):
x86/entry: Add auxiliary pt_regs space
x86/mm: Store CPU info on exception entry

Rik van Riel (1):
x86,mm: print likely CPU at segfault time

arch/x86/Kconfig | 4 ++++
arch/x86/entry/calling.h | 19 +++++++++++++++++++
arch/x86/entry/entry_64.S | 22 ++++++++++++++++++++++
arch/x86/entry/entry_64_compat.S | 6 ++++++
arch/x86/include/asm/ptrace.h | 19 +++++++++++++++++++
arch/x86/kernel/asm-offsets_64.c | 15 +++++++++++++++
arch/x86/kernel/head_64.S | 6 ++++++
arch/x86/mm/fault.c | 18 ++++++++++++++++++
8 files changed, 109 insertions(+)


base-commit: d4252071b97d2027d246f6a82cbee4d52f618b47
--
2.35.3


2022-08-11 03:20:15

by Ira Weiny

[permalink] [raw]
Subject: [PATCH 3/3] x86/mm: Store CPU info on exception entry

From: Ira Weiny <[email protected]>

x86 has auxiliary pt_regs space available to store information on the
stack during exceptions. This information is easier to obtain and store
within C code.

The CPU information of a page fault is useful in determining where bad
CPUs are in a large data center.

Store the CPU on page fault entry and use it later.

Cc: Rik van Riel <[email protected]>
Suggested-by: Borislav Petkov <[email protected]>
Suggested-by: Dave Hansen <[email protected]>
Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: Ira Weiny <[email protected]>

---
Changes from RFC:
New patch combining 2 and 5 from original series and modified.
Boris/Thomas - eliminate generic calls to save the cpu and call
only from exc_page_fault
---
arch/x86/Kconfig | 2 +-
arch/x86/include/asm/ptrace.h | 1 +
arch/x86/mm/fault.c | 12 ++++++++++--
3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5a0b6ee49cb4..e4a04406be2c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1874,7 +1874,7 @@ config X86_INTEL_MEMORY_PROTECTION_KEYS

config ARCH_HAS_PTREGS_AUXILIARY
depends on X86_64
- bool
+ def_bool y

choice
prompt "TSX enable mode"
diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
index 5a9c85893459..b403b469996f 100644
--- a/arch/x86/include/asm/ptrace.h
+++ b/arch/x86/include/asm/ptrace.h
@@ -97,6 +97,7 @@ struct pt_regs {
* ARCH_HAS_PTREGS_AUXILIARY. Failure to do so will result in a build failure.
*/
struct pt_regs_auxiliary {
+ int cpu;
};

struct pt_regs_extended {
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index dbc6a2e08a96..0aa420cd7874 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -768,9 +768,9 @@ static inline void
show_signal_msg(struct pt_regs *regs, unsigned long error_code,
unsigned long address, struct task_struct *tsk)
{
+ struct pt_regs_auxiliary *aux_pt_regs = &to_extended_pt_regs(regs)->aux;
const char *loglvl = task_pid_nr(tsk) > 1 ? KERN_INFO : KERN_EMERG;
- /* This is a racy snapshot, but it's better than nothing. */
- int cpu = raw_smp_processor_id();
+ int cpu = aux_pt_regs->cpu;

if (!unhandled_signal(tsk, SIGSEGV))
return;
@@ -1507,6 +1507,13 @@ handle_page_fault(struct pt_regs *regs, unsigned long error_code,
}
}

+noinstr static void aux_pt_regs_save_cpu(struct pt_regs *regs)
+{
+ struct pt_regs_auxiliary *aux_pt_regs = &to_extended_pt_regs(regs)->aux;
+
+ aux_pt_regs->cpu = raw_smp_processor_id();
+}
+
DEFINE_IDTENTRY_RAW_ERRORCODE(exc_page_fault)
{
unsigned long address = read_cr2();
@@ -1550,6 +1557,7 @@ DEFINE_IDTENTRY_RAW_ERRORCODE(exc_page_fault)
*/
state = irqentry_enter(regs);

+ aux_pt_regs_save_cpu(regs);
instrumentation_begin();
handle_page_fault(regs, error_code, address);
instrumentation_end();
--
2.35.3

2022-09-02 03:20:44

by Ira Weiny

[permalink] [raw]
Subject: Re: [PATCH 0/3] Print CPU at segfault time

On Wed, Aug 10, 2022 at 07:49:00PM -0700, Ira wrote:
> From: Ira Weiny <[email protected]>

Any feedback on this series? Or has Rik's patches been accepted instead?

Ira

>
> Changes from RFC:
> Drop patch 1 as I misunderstood the code and it is not needed for this
> use case
> Combine patch 2 and 5 into a patch 3 which stores the CPU only for page
> faults
>
> Rik reported that the knowledge of which CPU's are seeing faults can help in
> determining which CPUs are failing in a large data center.[1]
>
> Storing the CPU at exception entry time allows this print to report the CPU
> which actually took the exception. This still may not be the CPU which is
> failing but it should be closer.
>
> Dave and Boris recognized that the auxiliary pt_regs work I did for the PKS
> series could help to store this value and avoid passing the CPU throughout the
> fault handler call stack.
>
> The patches I sent for RFC had a lot more overhead than is needed to report the
> CPU in a page fault.[2] After the discussions the generic save/restore of the
> auxiliary pt_regs is overkill for the current use case. Skip that overhead and
> only store the CPU in the page fault path.
>
> [1] https://lore.kernel.org/all/[email protected]/
> [2] https://lore.kernel.org/lkml/[email protected]/
>
> Ira Weiny (2):
> x86/entry: Add auxiliary pt_regs space
> x86/mm: Store CPU info on exception entry
>
> Rik van Riel (1):
> x86,mm: print likely CPU at segfault time
>
> arch/x86/Kconfig | 4 ++++
> arch/x86/entry/calling.h | 19 +++++++++++++++++++
> arch/x86/entry/entry_64.S | 22 ++++++++++++++++++++++
> arch/x86/entry/entry_64_compat.S | 6 ++++++
> arch/x86/include/asm/ptrace.h | 19 +++++++++++++++++++
> arch/x86/kernel/asm-offsets_64.c | 15 +++++++++++++++
> arch/x86/kernel/head_64.S | 6 ++++++
> arch/x86/mm/fault.c | 18 ++++++++++++++++++
> 8 files changed, 109 insertions(+)
>
>
> base-commit: d4252071b97d2027d246f6a82cbee4d52f618b47
> --
> 2.35.3
>