Re: [PATCH 10/24] preempt: Introduce HAS_SEPARATE_PREEMPT_RESCHED_BITS
Boqun Feng <[email protected]> Mon, 3 Aug 2026 08:04:59 -0700
| Newsgroups | org.kernel.vger.rust-for-linux,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Mon, Aug 03, 2026 at 01:38:17PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 31, 2026 at 01:30:11PM -0700, Boqun Feng wrote:
>
> > diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h
> > index 1220656f3370..12353eeebc52 100644
> > --- a/arch/x86/include/asm/preempt.h
> > +++ b/arch/x86/include/asm/preempt.h
>
> > @@ -24,18 +34,26 @@ DECLARE_PER_CPU_CACHE_HOT(int, __preempt_count);
> > */
> > static __always_inline int preempt_count(void)
> > {
> > - return raw_cpu_read_4(__preempt_count) & ~PREEMPT_NEED_RESCHED;
> > + return __pc_op(read, __preempt_count) & ~PREEMPT_NEED_RESCHED;
> > }
> >
> > -static __always_inline void preempt_count_set(int pc)
> > +/*
> > + * unsigned long preempt count parameter works for both 32bit and 64bit cases:
> > + *
> > + * - For 32bit, "int" (the return of preempt_count()) and "unsigned long" have
> > + * the same size.
> > + * - For 64bit, the effective bits of a preempt count sits in 32bit, and we
> > + * reserve the NEED_RESCHED bit from the old count.
> > + */
>
> The 64bit comment doesn't really make sense to me.
>
Ah, I meant "preserve" instead of "reserve"..
+/*
+ * unsigned long preempt count parameter works for both 32bit and 64bit cases:
+ *
+ * - For 32bit, "int" (the return of preempt_count()) and "unsigned long" have
+ * the same size.
+ * - For 64bit, the effective bits of a preempt count sits in 32bit, and we
+ * preserve the NEED_RESCHED bit from the old count.
+ */
It means we won't change the NEED_RESCHED bit regardless that `pc` has
it set or not.
> > +static __always_inline void preempt_count_set(unsigned long pc)
> > {
> > - int old, new;
> > + unsigned long old, new;
> >
> > - old = raw_cpu_read_4(__preempt_count);
> > + old = __pc_op(read, __preempt_count);
> > do {
> > new = (old & PREEMPT_NEED_RESCHED) |
> > (pc & ~PREEMPT_NEED_RESCHED);
> > - } while (!raw_cpu_try_cmpxchg_4(__preempt_count, &old, new));
> > + } while (!__pc_op(try_cmpxchg, __preempt_count, &old, new));
> > }
>
> > diff --git a/include/linux/preempt.h b/include/linux/preempt.h
> > index 33fc4c814a9f..87d5367f986c 100644
> > --- a/include/linux/preempt.h
> > +++ b/include/linux/preempt.h
> > @@ -30,18 +30,20 @@
> > * NMI nesting depth is tracked in a separate per-CPU variable
> > * (nmi_nesting) to save bits in preempt_count.
> > *
> > - * PREEMPT_MASK: 0x000000ff
> > - * SOFTIRQ_MASK: 0x0000ff00
> > - * HARDIRQ_DISABLE_MASK: 0x00ff0000
> > - * HARDIRQ_MASK: 0x0f000000
> > - * NMI_MASK: 0x10000000
> > - * PREEMPT_NEED_RESCHED: 0x80000000
> > + * 32bit HAS_SEPARATE_PREEMPT_RESCHED_BITS
> > + *
> > + * PREEMPT_MASK: 0x000000ff 0x00000000000000ff
> > + * SOFTIRQ_MASK: 0x0000ff00 0x000000000000ff00
> > + * HARDIRQ_DISABLE_MASK: 0x00ff0000 0x0000000000ff0000
> > + * HARDIRQ_MASK: 0x0f000000 0x000000000f000000
> > + * NMI_MASK: 0x10000000 0x00000000f0000000
> > + * PREEMPT_NEED_RESCHED: 0x80000000 0x8000000000000000
> > */
>
> Perhaps add a comment about how HAS_SEPARATE_PREEMPT_RESCHED_BITS really
> is about having PREEMPT_NEED_RESCHED in its own word, rather than
> preempt_count() being 64bit.
>
> Because as presented it is very easy to confuse these two options.
> Ideally it would explain the LOAD-STORE issue with NEED_RESCHED and
> point to ARM64 or something.
How about organizing the comments as following:
+ *
+ * PREEMPT_MASK: 0x000000ff
+ * SOFTIRQ_MASK: 0x0000ff00
+ * HARDIRQ_DISABLE_MASK: 0x00ff0000
+ * HARDIRQ_MASK: 0x0f000000
+ *
+ * Depending on HAS_SEPARATE_PREEMPT_RESCHED_BITS, NEED_RESCHED bit
+ * is put in a separate 32bits.
+ *
+ * HAS_SEPARATE_PREEMPT_RESCHED_BITS=n:
+ *
+ * NMI_MASK: 0x10000000
+ * PREEMPT_NEED_RESCHED: 0x80000000
+ *
+ * HAS_SEPARATE_PREEMPT_RESCHED_BITS=y:
+ *
+ * (NMI_MASK can use all the 4 bits)
+ *
+ * NMI_MASK: 0xf0000000
Thoughts?
Regards,
Boqun