[PATCH 1/1] sched.h: silent false ATOMIC_SLEEP warning from cond_resched

[PATCH 1/1] sched.h: silent false ATOMIC_SLEEP warning from cond_resched

[Hariom Panthi]

In case of (CONFIG_PREEMPTION && !CONFIG_PREEMPT_DYNAMIC),
cond_reched() is not sleeping.

Thus remove __might_resched in that cases.

Co-developed-by: Maninder Singh <maninder1.s@samsung.com>
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
Signed-off-by: Hariom Panthi <hariom1.p@samsung.com>
---
 include/linux/sched.h | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index c6009faf8da5..a748ea8a435c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2067,6 +2067,7 @@ static inline int _cond_resched(void)
 
 #endif /* PREEMPT_DYNAMIC && CONFIG_HAVE_PREEMPT_DYNAMIC_CALL */
 
+#define __might_resched_possible	__might_resched
 #else /* CONFIG_PREEMPTION && !CONFIG_PREEMPT_DYNAMIC */
 
 static inline int _cond_resched(void)
@@ -2075,11 +2076,13 @@ static inline int _cond_resched(void)
 	return 0;
 }
 
+static inline void __might_resched_possible(const char *file, int line, unsigned int offsets) {}
+
 #endif /* !CONFIG_PREEMPTION || CONFIG_PREEMPT_DYNAMIC */
 
-#define cond_resched() ({			\
-	__might_resched(__FILE__, __LINE__, 0);	\
-	_cond_resched();			\
+#define cond_resched() ({					\
+	__might_resched_possible(__FILE__, __LINE__, 0);	\
+	_cond_resched();					\
 })
 
 extern int __cond_resched_lock(spinlock_t *lock);
@@ -2105,19 +2108,19 @@ extern int __cond_resched_rwlock_write(rwlock_t *lock);
 	(PREEMPT_LOCK_OFFSET + (1U << MIGHT_RESCHED_RCU_SHIFT))
 #endif
 
-#define cond_resched_lock(lock) ({						\
-	__might_resched(__FILE__, __LINE__, PREEMPT_LOCK_RESCHED_OFFSETS);	\
-	__cond_resched_lock(lock);						\
+#define cond_resched_lock(lock) ({							\
+	__might_resched_possible(__FILE__, __LINE__, PREEMPT_LOCK_RESCHED_OFFSETS);	\
+	__cond_resched_lock(lock);							\
 })
 
-#define cond_resched_rwlock_read(lock) ({					\
-	__might_resched(__FILE__, __LINE__, PREEMPT_LOCK_RESCHED_OFFSETS);	\
-	__cond_resched_rwlock_read(lock);					\
+#define cond_resched_rwlock_read(lock) ({						\
+	__might_resched_possible(__FILE__, __LINE__, PREEMPT_LOCK_RESCHED_OFFSETS);	\
+	__cond_resched_rwlock_read(lock);						\
 })
 
-#define cond_resched_rwlock_write(lock) ({					\
-	__might_resched(__FILE__, __LINE__, PREEMPT_LOCK_RESCHED_OFFSETS);	\
-	__cond_resched_rwlock_write(lock);					\
+#define cond_resched_rwlock_write(lock) ({						\
+	__might_resched_possible(__FILE__, __LINE__, PREEMPT_LOCK_RESCHED_OFFSETS);	\
+	__cond_resched_rwlock_write(lock);						\
 })
 
 static __always_inline bool need_resched(void)
-- 
2.25.1

Re: [PATCH 1/1] sched.h: silent false ATOMIC_SLEEP warning from cond_resched

[Peter Zijlstra]

On Thu, Oct 10, 2024 at 08:56:53AM +0530, Hariom Panthi wrote:
> In case of (CONFIG_PREEMPTION && !CONFIG_PREEMPT_DYNAMIC),
> cond_reched() is not sleeping.
> 
> Thus remove __might_resched in that cases.

*why* ? It's still a valid site to do the atomic_sleep testing, no?

RE: [PATCH 1/1] sched.h: silent false ATOMIC_SLEEP warning from cond_resched

[Maninder Singh]

Hi,

> On Thu, Oct 10, 2024 at 08:56:53AM +0530, Hariom Panthi wrote:
> > In case of (CONFIG_PREEMPTION && !CONFIG_PREEMPT_DYNAMIC),
> > cond_reched() is not sleeping.
> > 
> > Thus remove __might_resched in that cases.
> 
> *why* ? It's still a valid site to do the atomic_sleep testing, no?

In our case there was a call to vunmap_pmd_range from __do_softirq,
and vunmap_pmd_range is not actually sleeping call, but because of
cond_resched it was giving warning with DEBUG_ATOMIC_SLEEP.

and cond_resched in case of CONFIG_PREEMPTION is empty function with below change:

commit 35a773a07926a22bf19d77ee00024522279c4e68
Author: Peter Zijlstra <peterz@infradead.org>
Date:   Mon Sep 19 12:57:53 2016 +0200

    sched/core: Avoid _cond_resched() for PREEMPT=y

    On fully preemptible kernels _cond_resched() is pointless, so avoid
    emitting any code for it.


So we thought it should not give warning also in this case.


Thanks,
Maninder Singh

Re: [PATCH 1/1] sched.h: silent false ATOMIC_SLEEP warning from cond_resched

[Peter Zijlstra]

On Thu, Oct 10, 2024 at 03:39:40PM +0530, Maninder Singh wrote:
> Hi,
> 
> > On Thu, Oct 10, 2024 at 08:56:53AM +0530, Hariom Panthi wrote:
> > > In case of (CONFIG_PREEMPTION && !CONFIG_PREEMPT_DYNAMIC),
> > > cond_reched() is not sleeping.
> > > 
> > > Thus remove __might_resched in that cases.
> > 
> > *why* ? It's still a valid site to do the atomic_sleep testing, no?
> 
> In our case there was a call to vunmap_pmd_range from __do_softirq,
> and vunmap_pmd_range is not actually sleeping call, but because of
> cond_resched it was giving warning with DEBUG_ATOMIC_SLEEP.
> 
> and cond_resched in case of CONFIG_PREEMPTION is empty function with below change:

By still having that warning we helpfully tell you your code is broken
for that other .config. Code should be good irrespective of the .config
used, right?

RE: [PATCH 1/1] sched.h: silent false ATOMIC_SLEEP warning from cond_resched

[Maninder Singh]

> > 
> > In our case there was a call to vunmap_pmd_range from __do_softirq,
> > and vunmap_pmd_range is not actually sleeping call, but because of
> > cond_resched it was giving warning with DEBUG_ATOMIC_SLEEP.
> > 
> > and cond_resched in case of CONFIG_PREEMPTION is empty function with below change:
> 
> By still having that warning we helpfully tell you your code is broken
> for that other .config. Code should be good irrespective of the .config
> used, right?

But isn't it un-necessary to report a warning for a particular .config set,
when in actual that issue can not happen and code was written accordingly.

Because for other .config it will be a total different system and user can change 
code design for that.


But you better know about it, so it is your call :)

Thanks,
Maninder Singh