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)))
