Re: [PATCH v1] [semaphore] Removed redundant code from semaphore's down family of function

[Peter Zijlstra]

On Mon, Aug 26, 2019 at 04:14:36PM +0200, Peter Zijlstra wrote:
> (XXX, we should probably move the schedule_timeout() thing into its own
> patch)

A better version here...

---
Subject: sched,time: Allow better constprop/DCE for schedule_timeout()

If timeout is constant and MAX_SCHEDULE_TIMEOUT, it would be nice to
allow to optimize away everything timeout.

Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
---
 include/linux/sched.h | 13 ++++++++++++-
 kernel/time/timer.c   | 52 ++++++++++++++++++++++++---------------------------
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index f0edee94834a..6003e96bce52 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -214,7 +214,18 @@ extern void scheduler_tick(void);
 
 #define	MAX_SCHEDULE_TIMEOUT		LONG_MAX
 
-extern long schedule_timeout(long timeout);
+extern long __schedule_timeout(long timeout);
+
+static inline long schedule_timeout(long timeout)
+{
+	if (__builtin_constant_p(timeout) && timeout == MAX_SCHEDULE_TIMEOUT) {
+		schedule();
+		return timeout;
+	}
+
+	return __schedule_timeout(timeout);
+}
+
 extern long schedule_timeout_interruptible(long timeout);
 extern long schedule_timeout_killable(long timeout);
 extern long schedule_timeout_uninterruptible(long timeout);
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 0e315a2e77ae..912ae56b96b8 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1851,38 +1851,34 @@ static void process_timeout(struct timer_list *t)
  * jiffies will be returned.  In all cases the return value is guaranteed
  * to be non-negative.
  */
-signed long __sched schedule_timeout(signed long timeout)
+signed long __sched __schedule_timeout(signed long timeout)
 {
 	struct process_timer timer;
 	unsigned long expire;
 
-	switch (timeout)
-	{
-	case MAX_SCHEDULE_TIMEOUT:
-		/*
-		 * These two special cases are useful to be comfortable
-		 * in the caller. Nothing more. We could take
-		 * MAX_SCHEDULE_TIMEOUT from one of the negative value
-		 * but I' d like to return a valid offset (>=0) to allow
-		 * the caller to do everything it want with the retval.
-		 */
+	/*
+	 * We could take MAX_SCHEDULE_TIMEOUT from one of the negative values,
+	 * but I'd like to return a valid offset (>= 0) to allow the caller to
+	 * do everything it wants with the retval.
+	 */
+	if (timeout == MAX_SCHEDULE_TIMEOUT) {
 		schedule();
-		goto out;
-	default:
-		/*
-		 * Another bit of PARANOID. Note that the retval will be
-		 * 0 since no piece of kernel is supposed to do a check
-		 * for a negative retval of schedule_timeout() (since it
-		 * should never happens anyway). You just have the printk()
-		 * that will tell you if something is gone wrong and where.
-		 */
-		if (timeout < 0) {
-			printk(KERN_ERR "schedule_timeout: wrong timeout "
+		return timeout;
+	}
+
+	/*
+	 * Another bit of PARANOID. Note that the retval will be 0 since no
+	 * piece of kernel is supposed to do a check for a negative retval of
+	 * schedule_timeout() (since it should never happens anyway). You just
+	 * have the printk() that will tell you if something is gone wrong and
+	 * where.
+	 */
+	if (timeout < 0) {
+		printk(KERN_ERR "schedule_timeout: wrong timeout "
 				"value %lx\n", timeout);
-			dump_stack();
-			current->state = TASK_RUNNING;
-			goto out;
-		}
+		dump_stack();
+		current->state = TASK_RUNNING;
+		goto out;
 	}
 
 	expire = timeout + jiffies;
@@ -1898,10 +1894,10 @@ signed long __sched schedule_timeout(signed long timeout)
 
 	timeout = expire - jiffies;
 
- out:
+out:
 	return timeout < 0 ? 0 : timeout;
 }
-EXPORT_SYMBOL(schedule_timeout);
+EXPORT_SYMBOL(__schedule_timeout);
 
 /*
  * We can use __set_current_state() here because schedule_timeout() calls