[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZxrCrlg1XvaTtJ1I@boqun-archlinux>
Date: Thu, 24 Oct 2024 14:57:02 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: Peter Zijlstra <peterz@...radead.org>,
Dirk Behme <dirk.behme@...il.com>, Lyude Paul <lyude@...hat.com>,
rust-for-linux@...r.kernel.org, Danilo Krummrich <dakr@...hat.com>,
airlied@...hat.com, Ingo Molnar <mingo@...hat.com>, will@...nel.org,
Waiman Long <longman@...hat.com>, linux-kernel@...r.kernel.org,
Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>, wedsonaf@...il.com,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...sung.com>, aliceryhl@...gle.com,
Trevor Gross <tmgross@...ch.edu>
Subject: Re: [POC 1/6] irq & spin_lock: Add counted interrupt
disabling/enabling
On Thu, Oct 24, 2024 at 07:22:19PM +0200, Thomas Gleixner wrote:
> On Thu, Oct 24 2024 at 12:05, Peter Zijlstra wrote:
> > On Wed, Oct 23, 2024 at 10:38:38PM +0200, Thomas Gleixner wrote:
> >> But if we want to support insanity then we make preempt count 64 bit and
> >> be done with it. But no, I don't think that encouraging insanity is a
> >> good thing.
> >
> > The problem is that in most release builds the overflow will be silent
> > and cause spurious weirdness that is a pain in the arse to debug :/
> >
> > That is my only concern -- making insane code crash hard is good, making
> > it silently mostly work but cause random weirdness is not.
>
> I wish we could come up with a lightweight check for that.
>
Since the preempt part takes exactly one byte in the preempt counter,
maybe we could use a "incb + jo"?
For example as below, note that since I used OF here, so it will try the
byte as s8 therefore overflow at 128, so 127 is the max level of
nesting.
Would this be a relatively lightweight check?
Regards,
Boqun
--------------------------->8
diff --git a/arch/x86/include/asm/current.h b/arch/x86/include/asm/current.h
index bf5953883ec3..c233b7703194 100644
--- a/arch/x86/include/asm/current.h
+++ b/arch/x86/include/asm/current.h
@@ -16,7 +16,10 @@ struct pcpu_hot {
union {
struct {
struct task_struct *current_task;
- int preempt_count;
+ union {
+ int preempt_count;
+ u8 preempt_bytes[4];
+ };
int cpu_number;
#ifdef CONFIG_MITIGATION_CALL_DEPTH_TRACKING
u64 call_depth;
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index c55a79d5feae..8d3725f8f2c7 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -251,6 +251,17 @@ do { \
percpu_binary_op(size, qual, "add", var, val); \
} while (0)
+#define percpu_check_inc(size, qual, _var) \
+({ \
+ bool overflow; \
+ \
+ asm qual (__pcpu_op1_##size("inc", __percpu_arg([var])) \
+ CC_SET(o) \
+ : CC_OUT(o) (overflow), [var] "+m" (__my_cpu_var(_var))); \
+ \
+ overflow; \
+})
+
/*
* Add return operation
*/
@@ -488,6 +499,7 @@ do { \
#define this_cpu_read_stable_4(pcp) __raw_cpu_read_stable(4, pcp)
#define raw_cpu_add_1(pcp, val) percpu_add_op(1, , (pcp), val)
+#define raw_cpu_check_inc_1(pcp) percpu_check_inc(1, , (pcp))
#define raw_cpu_add_2(pcp, val) percpu_add_op(2, , (pcp), val)
#define raw_cpu_add_4(pcp, val) percpu_add_op(4, , (pcp), val)
#define raw_cpu_and_1(pcp, val) percpu_binary_op(1, , "and", (pcp), val)
diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
index 919909d8cb77..a39cf8c0fc8b 100644
--- a/arch/x86/include/asm/preempt.h
+++ b/arch/x86/include/asm/preempt.h
@@ -2,6 +2,7 @@
#ifndef __ASM_PREEMPT_H
#define __ASM_PREEMPT_H
+#include <asm/bug.h>
#include <asm/rmwcc.h>
#include <asm/percpu.h>
#include <asm/current.h>
@@ -76,7 +77,12 @@ static __always_inline bool test_preempt_need_resched(void)
static __always_inline void __preempt_count_add(int val)
{
- raw_cpu_add_4(pcpu_hot.preempt_count, val);
+ if (__builtin_constant_p(val) && val == 1) {
+ /* Panic if overflow */
+ BUG_ON(raw_cpu_check_inc_1(pcpu_hot.preempt_bytes[0]));
+ } else {
+ raw_cpu_add_4(pcpu_hot.preempt_count, val);
+ }
}
static __always_inline void __preempt_count_sub(int val)
Powered by blists - more mailing lists