Re: [PATCH v2 7/9] sched: define TIF_ALLOW_RESCHED

[Peter Zijlstra]

On Wed, Aug 30, 2023 at 11:49:56AM -0700, Ankur Arora wrote:

> +#ifdef TIF_RESCHED_ALLOW
> +/*
> + * allow_resched() .. disallow_resched() demarcate a preemptible section.
> + *
> + * Used around primitives where it might not be convenient to periodically
> + * call cond_resched().
> + */
> +static inline void allow_resched(void)
> +{
> +	might_sleep();
> +	set_tsk_thread_flag(current, TIF_RESCHED_ALLOW);

So the might_sleep() ensures we're not currently having preemption
disabled; but there's nothing that ensures we don't do stupid things
like:

	allow_resched();
	spin_lock();
	...
	spin_unlock();
	disallow_resched();

Which on a PREEMPT_COUNT=n build will cause preemption while holding the
spinlock. I think something like the below will cause sufficient
warnings to avoid growing patterns like that.


Index: linux-2.6/kernel/sched/core.c
===================================================================
--- linux-2.6.orig/kernel/sched/core.c
+++ linux-2.6/kernel/sched/core.c
@@ -5834,6 +5834,13 @@ void preempt_count_add(int val)
 {
 #ifdef CONFIG_DEBUG_PREEMPT
 	/*
+	 * Disabling preemption under TIF_RESCHED_ALLOW doesn't
+	 * work for PREEMPT_COUNT=n builds.
+	 */
+	if (WARN_ON(resched_allowed()))
+		return;
+
+	/*
 	 * Underflow?
 	 */
 	if (DEBUG_LOCKS_WARN_ON((preempt_count() < 0)))