All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wen Yang <wen.yang@linux.dev>
To: Bjorn Andersson <andersson@kernel.org>
Cc: Wen Yang <wen.yang@linux.dev>,
	Baolin Wang <baolin.wang@linux.alibaba.com>,
	Dave Young <dyoung@redhat.com>,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] hwspinlock: fix some comments about 'will never sleep'
Date: Thu, 26 Sep 2024 10:30:28 +0800	[thread overview]
Message-ID: <20240926023028.774580-1-wen.yang@linux.dev> (raw)

Both __hwspin_trylock and __hwspin_unlock use hwlock->lock,
with a special annotation:
function will never sleep.
However, this requirement is not fulfilled on PREEMPT_RT.

Bjorn said:
: "will never sleep" comment expresses that the function can be called
: in atomic or irq context, not necessarily that it must not sleep.

This patch fixes these comments.

Suggested-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Dave Young <dyoung@redhat.com>
Cc: linux-remoteproc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 Documentation/locking/hwspinlock.rst | 64 ++++++++++++++--------------
 drivers/hwspinlock/hwspinlock_core.c | 25 +++++------
 2 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/Documentation/locking/hwspinlock.rst b/Documentation/locking/hwspinlock.rst
index 2ffaa3cbd63f..9d20823a21e7 100644
--- a/Documentation/locking/hwspinlock.rst
+++ b/Documentation/locking/hwspinlock.rst
@@ -103,14 +103,14 @@ Should be called from a process context (might sleep).
 Lock a previously-assigned hwspinlock with a timeout limit (specified in
 msecs). If the hwspinlock is already taken, the function will busy loop
 waiting for it to be released, but give up when the timeout elapses.
-Upon a successful return from this function, preemption is disabled so
-the caller must not sleep, and is advised to release the hwspinlock as
-soon as possible, in order to minimize remote cores polling on the
-hardware interconnect.
+Upon a successful return from this function, preemption is disabled on
+non-PREEMPT_RT kernels, so the caller must not sleep, and is advised to
+release the hwspinlock as soon as possible, in order to minimize remote
+cores polling on the hardware interconnect.
 
 Returns 0 when successful and an appropriate error code otherwise (most
 notably -ETIMEDOUT if the hwspinlock is still busy after timeout msecs).
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -120,12 +120,12 @@ Lock a previously-assigned hwspinlock with a timeout limit (specified in
 msecs). If the hwspinlock is already taken, the function will busy loop
 waiting for it to be released, but give up when the timeout elapses.
 Upon a successful return from this function, preemption and the local
-interrupts are disabled, so the caller must not sleep, and is advised to
-release the hwspinlock as soon as possible.
+interrupts are disabled on non-PREEMPT_RT kernels, so the caller must not
+sleep, and is advised to release the hwspinlock as soon as possible.
 
 Returns 0 when successful and an appropriate error code otherwise (most
 notably -ETIMEDOUT if the hwspinlock is still busy after timeout msecs).
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -137,13 +137,13 @@ msecs). If the hwspinlock is already taken, the function will busy loop
 waiting for it to be released, but give up when the timeout elapses.
 Upon a successful return from this function, preemption is disabled,
 local interrupts are disabled and their previous state is saved at the
-given flags placeholder. The caller must not sleep, and is advised to
-release the hwspinlock as soon as possible.
+given flags placeholder on non-PREEMPT_RT kernels. The caller must not sleep,
+and is advised to release the hwspinlock as soon as possible.
 
 Returns 0 when successful and an appropriate error code otherwise (most
 notably -ETIMEDOUT if the hwspinlock is still busy after timeout msecs).
 
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -160,7 +160,7 @@ or sleepable operations under the hardware lock.
 Returns 0 when successful and an appropriate error code otherwise (most
 notably -ETIMEDOUT if the hwspinlock is still busy after timeout msecs).
 
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -176,7 +176,7 @@ value shall not exceed a few msecs.
 Returns 0 when successful and an appropriate error code otherwise (most
 notably -ETIMEDOUT if the hwspinlock is still busy after timeout msecs).
 
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -186,14 +186,14 @@ The function will never sleep.
 Attempt to lock a previously-assigned hwspinlock, but immediately fail if
 it is already taken.
 
-Upon a successful return from this function, preemption is disabled so
-caller must not sleep, and is advised to release the hwspinlock as soon as
-possible, in order to minimize remote cores polling on the hardware
-interconnect.
+Upon a successful return from this function, preemption is disabled on
+non-PREEMPT_RT kernels so caller must not sleep and is advised to release
+the hwspinlock as soon as possible, in order to minimize remote cores polling
+on the hardware interconnect.
 
 Returns 0 on success and an appropriate error code otherwise (most
 notably -EBUSY if the hwspinlock was already taken).
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -204,13 +204,13 @@ Attempt to lock a previously-assigned hwspinlock, but immediately fail if
 it is already taken.
 
 Upon a successful return from this function, preemption and the local
-interrupts are disabled so caller must not sleep, and is advised to
-release the hwspinlock as soon as possible.
+interrupts are disabled on non-PREEMPT_RT kernels so caller must not sleep,
+and is advised to release the hwspinlock as soon as possible.
 
 Returns 0 on success and an appropriate error code otherwise (most
 notably -EBUSY if the hwspinlock was already taken).
 
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -221,12 +221,12 @@ it is already taken.
 
 Upon a successful return from this function, preemption is disabled,
 the local interrupts are disabled and their previous state is saved
-at the given flags placeholder. The caller must not sleep, and is advised
-to release the hwspinlock as soon as possible.
+at the given flags placeholder on non-PREEMPT_RT kernels. The caller must
+not sleep, and is advised to release the hwspinlock as soon as possible.
 
 Returns 0 on success and an appropriate error code otherwise (most
 notably -EBUSY if the hwspinlock was already taken).
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -241,7 +241,7 @@ or sleepable operations under the hardware lock.
 
 Returns 0 on success and an appropriate error code otherwise (most
 notably -EBUSY if the hwspinlock was already taken).
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -254,14 +254,14 @@ This function shall be called only from an atomic context.
 
 Returns 0 on success and an appropriate error code otherwise (most
 notably -EBUSY if the hwspinlock was already taken).
-The function will never sleep.
+The function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
   void hwspin_unlock(struct hwspinlock *hwlock);
 
 Unlock a previously-locked hwspinlock. Always succeed, and can be called
-from any context (the function never sleeps).
+from any context (the function never sleeps on a non-PREEMPT_RT kernel).
 
 .. note::
 
@@ -277,7 +277,8 @@ The caller should **never** unlock an hwspinlock which is already unlocked.
 
 Doing so is considered a bug (there is no protection against this).
 Upon a successful return from this function, preemption and local
-interrupts are enabled. This function will never sleep.
+interrupts are enabled on non-PREEMPT_RT kernels.
+This function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -290,7 +291,8 @@ The caller should **never** unlock an hwspinlock which is already unlocked.
 Doing so is considered a bug (there is no protection against this).
 Upon a successful return from this function, preemption is reenabled,
 and the state of the local interrupts is restored to the state saved at
-the given flags. This function will never sleep.
+the given flags on non-PREEMPT_RT kernels.
+This function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -300,7 +302,7 @@ Unlock a previously-locked hwspinlock.
 
 The caller should **never** unlock an hwspinlock which is already unlocked.
 Doing so is considered a bug (there is no protection against this).
-This function will never sleep.
+This function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
@@ -310,7 +312,7 @@ Unlock a previously-locked hwspinlock.
 
 The caller should **never** unlock an hwspinlock which is already unlocked.
 Doing so is considered a bug (there is no protection against this).
-This function will never sleep.
+This function will never sleep on a non-PREEMPT_RT kernel.
 
 ::
 
diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
index 6505261e6068..b602d6cf5e1b 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -73,10 +73,10 @@ static DEFINE_MUTEX(hwspinlock_tree_lock);
  * lock, they need one sleepable lock (like mutex) to protect the operations.
  *
  * If the mode is neither HWLOCK_IN_ATOMIC nor HWLOCK_RAW, upon a successful
- * return from this function, preemption (and possibly interrupts) is disabled,
- * so the caller must not sleep, and is advised to release the hwspinlock as
- * soon as possible. This is required in order to minimize remote cores polling
- * on the hardware interconnect.
+ * return from this function, preemption (and possibly interrupts) is disabled
+ * on non-PREEMPT_RT kernels, so the caller must not sleep, and is advised to
+ * release the hwspinlock as soon as possible. This is required in order to
+ * minimize remote cores polling on the hardware interconnect.
  *
  * The user decides whether local interrupts are disabled or not, and if yes,
  * whether he wants their previous state to be saved. It is up to the user
@@ -87,7 +87,7 @@ static DEFINE_MUTEX(hwspinlock_tree_lock);
  * Returns: %0 if we successfully locked the hwspinlock or -EBUSY if
  * the hwspinlock was already taken.
  *
- * This function will never sleep.
+ * This function will never sleep on a non-PREEMPT_RT kernel.
  */
 int __hwspin_trylock(struct hwspinlock *hwlock, int mode, unsigned long *flags)
 {
@@ -190,10 +190,10 @@ EXPORT_SYMBOL_GPL(__hwspin_trylock);
  * is handled with busy-waiting delays, hence shall not exceed few msecs.
  *
  * If the mode is neither HWLOCK_IN_ATOMIC nor HWLOCK_RAW, upon a successful
- * return from this function, preemption (and possibly interrupts) is disabled,
- * so the caller must not sleep, and is advised to release the hwspinlock as
- * soon as possible. This is required in order to minimize remote cores polling
- * on the hardware interconnect.
+ * return from this function, preemption (and possibly interrupts) is disabled
+ * on non-PREEMPT_RT kernels, so the caller must not sleep, and is advised to
+ * release the hwspinlock as soon as possible. This is required in order to
+ * minimize remote cores polling on the hardware interconnect.
  *
  * The user decides whether local interrupts are disabled or not, and if yes,
  * whether he wants their previous state to be saved. It is up to the user
@@ -204,7 +204,7 @@ EXPORT_SYMBOL_GPL(__hwspin_trylock);
  * error code otherwise (most notably -ETIMEDOUT if the @hwlock is still
  * busy after @timeout msecs).
  *
- * The function will never sleep.
+ * The function will never sleep on a non-PREEMPT_RT kernel.
  */
 int __hwspin_lock_timeout(struct hwspinlock *hwlock, unsigned int to,
 					int mode, unsigned long *flags)
@@ -253,7 +253,8 @@ EXPORT_SYMBOL_GPL(__hwspin_lock_timeout);
  * @flags: previous caller's interrupt state to restore (if requested)
  *
  * This function will unlock a specific hwspinlock, enable preemption and
- * (possibly) enable interrupts or restore their previous state.
+ * (possibly) enable interrupts or restore their previous state on
+ * non-PREEMPT_RT kernels.
  * @hwlock must be already locked before calling this function: it is a bug
  * to call unlock on a @hwlock that is already unlocked.
  *
@@ -263,7 +264,7 @@ EXPORT_SYMBOL_GPL(__hwspin_lock_timeout);
  * same way users decide between spin_unlock, spin_unlock_irq and
  * spin_unlock_irqrestore.
  *
- * The function will never sleep.
+ * The function will never sleep on a non-PREEMPT_RT kernel.
  */
 void __hwspin_unlock(struct hwspinlock *hwlock, int mode, unsigned long *flags)
 {
-- 
2.25.1


                 reply	other threads:[~2024-09-26  2:31 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

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=20240926023028.774580-1-wen.yang@linux.dev \
    --to=wen.yang@linux.dev \
    --cc=andersson@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=dyoung@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.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.