All of lore.kernel.org
 help / color / mirror / Atom feed
From: Satendra Singh Thakur <sst2005@gmail.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Satendra Singh Thakur <sst2005@gmail.com>,
	Satendra Singh Thakur <satendrasingh.thakur@hcl.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH v1] [semaphore] Removed redundant code from semaphore's down family of function
Date: Mon, 12 Aug 2019 19:18:59 +0530	[thread overview]
Message-ID: <20190812134859.16061-1-sst2005@gmail.com> (raw)
In-Reply-To: <20190812053014.27743-1-satendrasingh.thakur@hcl.com>

-The semaphore code has four funcs
down,
down_interruptible,
down_killable,
down_timeout
-These four funcs have almost similar code except that
they all call lower level function __down_xyz.
-This lower level func in-turn call inline func
__down_common with appropriate arguments.
-This patch creates a common macro for above family of funcs
so that duplicate code is eliminated.
-Also, __down_common has been made noinline so that code is
functionally similar to previous one
-For example, earlier down_killable would call __down_killable
, which in-turn would call inline func __down_common
Now, down_killable calls noinline __down_common directly
through a macro
-The funcs __down_interruptible, __down_killable etc have been
removed as they were just wrapper to __down_common

Signed-off-by: Satendra Singh Thakur <satendrasingh.thakur@hcl.com>
---
 v1: removed disclaimer appended automatically by company mail policy

 kernel/locking/semaphore.c | 107 +++++++++++++------------------------
 1 file changed, 38 insertions(+), 69 deletions(-)

diff --git a/kernel/locking/semaphore.c b/kernel/locking/semaphore.c
index d9dd94defc0a..0468bc335908 100644
--- a/kernel/locking/semaphore.c
+++ b/kernel/locking/semaphore.c
@@ -33,11 +33,33 @@
 #include <linux/spinlock.h>
 #include <linux/ftrace.h>
 
-static noinline void __down(struct semaphore *sem);
-static noinline int __down_interruptible(struct semaphore *sem);
-static noinline int __down_killable(struct semaphore *sem);
-static noinline int __down_timeout(struct semaphore *sem, long timeout);
+static noinline int __sched __down_common(struct semaphore *sem, long state,
+								long timeout);
 static noinline void __up(struct semaphore *sem);
+/**
+ * down_common - acquire the semaphore
+ * @sem: the semaphore to be acquired
+ * @state: new state of the task
+ * @timeout: either MAX_SCHEDULE_TIMEOUT or actual specified
+ * timeout
+ * Acquires the semaphore. If no more tasks are allowed to
+ * acquire the semaphore, calling this macro will put the task
+ * to sleep until the semaphore is released.
+ *
+ * This internally calls another func __down_common.
+ */
+#define down_common(sem, state, timeout)	\
+({	\
+	int ret = 0;	\
+	unsigned long flags;	\
+	raw_spin_lock_irqsave(&(sem)->lock, flags);	\
+	if (likely((sem)->count > 0))	\
+		(sem)->count--;	\
+	else	\
+		ret = __down_common(sem, state, timeout);	\
+	raw_spin_unlock_irqrestore(&(sem)->lock, flags);	\
+	ret;	\
+})
 
 /**
  * down - acquire the semaphore
@@ -52,14 +74,7 @@ static noinline void __up(struct semaphore *sem);
  */
 void down(struct semaphore *sem)
 {
-	unsigned long flags;
-
-	raw_spin_lock_irqsave(&sem->lock, flags);
-	if (likely(sem->count > 0))
-		sem->count--;
-	else
-		__down(sem);
-	raw_spin_unlock_irqrestore(&sem->lock, flags);
+	down_common(sem, TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
 }
 EXPORT_SYMBOL(down);
 
@@ -74,17 +89,7 @@ EXPORT_SYMBOL(down);
  */
 int down_interruptible(struct semaphore *sem)
 {
-	unsigned long flags;
-	int result = 0;
-
-	raw_spin_lock_irqsave(&sem->lock, flags);
-	if (likely(sem->count > 0))
-		sem->count--;
-	else
-		result = __down_interruptible(sem);
-	raw_spin_unlock_irqrestore(&sem->lock, flags);
-
-	return result;
+	return down_common(sem, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
 }
 EXPORT_SYMBOL(down_interruptible);
 
@@ -100,17 +105,7 @@ EXPORT_SYMBOL(down_interruptible);
  */
 int down_killable(struct semaphore *sem)
 {
-	unsigned long flags;
-	int result = 0;
-
-	raw_spin_lock_irqsave(&sem->lock, flags);
-	if (likely(sem->count > 0))
-		sem->count--;
-	else
-		result = __down_killable(sem);
-	raw_spin_unlock_irqrestore(&sem->lock, flags);
-
-	return result;
+	return down_common(sem, TASK_KILLABLE, MAX_SCHEDULE_TIMEOUT);
 }
 EXPORT_SYMBOL(down_killable);
 
@@ -154,17 +149,7 @@ EXPORT_SYMBOL(down_trylock);
  */
 int down_timeout(struct semaphore *sem, long timeout)
 {
-	unsigned long flags;
-	int result = 0;
-
-	raw_spin_lock_irqsave(&sem->lock, flags);
-	if (likely(sem->count > 0))
-		sem->count--;
-	else
-		result = __down_timeout(sem, timeout);
-	raw_spin_unlock_irqrestore(&sem->lock, flags);
-
-	return result;
+	return down_common(sem, TASK_UNINTERRUPTIBLE, timeout);
 }
 EXPORT_SYMBOL(down_timeout);
 
@@ -196,12 +181,15 @@ struct semaphore_waiter {
 	bool up;
 };
 
-/*
- * Because this function is inlined, the 'state' parameter will be
- * constant, and thus optimised away by the compiler.  Likewise the
- * 'timeout' parameter for the cases without timeouts.
+/**
+ * __down_common - Adds the current task to wait list
+ * puts the task to sleep until signal, timeout or up flag
+ * @sem: the semaphore to be acquired
+ * @state: the state of the calling task
+ * @timeout: either MAX_SCHEDULE_TIMEOUT or actual specified
+ * timeout
  */
-static inline int __sched __down_common(struct semaphore *sem, long state,
+static noinline int __sched __down_common(struct semaphore *sem, long state,
 								long timeout)
 {
 	struct semaphore_waiter waiter;
@@ -232,25 +220,6 @@ static inline int __sched __down_common(struct semaphore *sem, long state,
 	return -EINTR;
 }
 
-static noinline void __sched __down(struct semaphore *sem)
-{
-	__down_common(sem, TASK_UNINTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
-}
-
-static noinline int __sched __down_interruptible(struct semaphore *sem)
-{
-	return __down_common(sem, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
-}
-
-static noinline int __sched __down_killable(struct semaphore *sem)
-{
-	return __down_common(sem, TASK_KILLABLE, MAX_SCHEDULE_TIMEOUT);
-}
-
-static noinline int __sched __down_timeout(struct semaphore *sem, long timeout)
-{
-	return __down_common(sem, TASK_UNINTERRUPTIBLE, timeout);
-}
 
 static noinline void __sched __up(struct semaphore *sem)
 {
-- 
2.17.1


  parent reply	other threads:[~2019-08-12 13:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-12  5:31 [PATCH] [semaphore] Removed redundant code from semaphore's down family of function Satendra Singh Thakur
2019-08-12  9:12 ` Will Deacon
2019-08-12 13:48 ` Satendra Singh Thakur [this message]
2019-08-22 15:51   ` [PATCH v1] " Peter Zijlstra
2019-08-24  3:50     ` Satendra Singh Thakur
2019-08-26 14:14       ` Peter Zijlstra
2019-08-26 14:25         ` Peter Zijlstra
2019-08-27  0:06           ` sched,time: Allow better constprop/DCE for schedule_timeout() kbuild test robot
2019-08-27  1:26           ` kbuild test robot
2019-08-30 10:40           ` [PATCH v1] [semaphore] Removed redundant code from semaphore's down family of function Satendra Singh Thakur
2019-08-30 10:46             ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190812134859.16061-1-sst2005@gmail.com \
    --to=sst2005@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=satendrasingh.thakur@hcl.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.