Re: [POC 1/6] irq & spin_lock: Add counted interrupt disabling/enabling

[Boqun Feng <boqun.feng@gmail.com>]

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)