Re: [PATCH 3/7] hwspinlock: Introduce hwspin_lock_bust()

From: Bryan O'Donoghue
Date: Fri May 17 2024 - 04:08:04 EST


On 17/05/2024 00:58, Chris Lew wrote:
From: Richard Maina <quic_rmaina@xxxxxxxxxxx>

When a remoteproc crashes or goes down unexpectedly this can result in
a state where locks held by the remoteproc will remain locked possibly
resulting in deadlock. This new API hwspin_lock_bust() allows
hwspinlock implementers to define a bust operation for freeing previously
acquired hwspinlocks after verifying ownership of the acquired lock.

Signed-off-by: Richard Maina <quic_rmaina@xxxxxxxxxxx>
Signed-off-by: Chris Lew <quic_clew@xxxxxxxxxxx>
---
Documentation/locking/hwspinlock.rst | 11 +++++++++++
drivers/hwspinlock/hwspinlock_core.c | 30 +++++++++++++++++++++++++++++-

Shouldn't this be added to drivers/hwspinlock/qcom_hwspinlock.c ?

drivers/hwspinlock/hwspinlock_internal.h | 3 +++
include/linux/hwspinlock.h | 6 ++++++
4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/Documentation/locking/hwspinlock.rst b/Documentation/locking/hwspinlock.rst
index c1c2c827b575..6ee94cc6d3b7 100644
--- a/Documentation/locking/hwspinlock.rst
+++ b/Documentation/locking/hwspinlock.rst
@@ -85,6 +85,17 @@ is already free).
Should be called from a process context (might sleep).
+::
+
+ int hwspin_lock_bust(struct hwspinlock *hwlock, unsigned int id);

I don't think this is a geat name "bust" looks alot like "burst" and I don't think aligns well with the established namespace.

Why not simply qcom_hwspinlock_unlock_force() - which is what you are doing - forcing the hw spinlock to unlock.

---
bod