[PATCH 1/3] refcount: move kdoc to header definition

[PATCH 1/3] refcount: move kdoc to header definition

[Alexander Aring]

This patch moves the kdoc for refcount_dec_and_lock functionality to
it's header prototype declarations.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 include/linux/refcount.h | 48 ++++++++++++++++++++++++++++++++++++++++
 lib/refcount.c           | 45 -------------------------------------
 2 files changed, 48 insertions(+), 45 deletions(-)

diff --git a/include/linux/refcount.h b/include/linux/refcount.h
index a62fcca97486..741cc6295f54 100644
--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -361,8 +361,56 @@ static inline void refcount_dec(refcount_t *r)
 
 extern __must_check bool refcount_dec_if_one(refcount_t *r);
 extern __must_check bool refcount_dec_not_one(refcount_t *r);
+
+/**
+ * refcount_dec_and_mutex_lock - return holding mutex if able to decrement
+ *                               refcount to 0
+ * @r: the refcount
+ * @lock: the mutex to be locked
+ *
+ * Similar to atomic_dec_and_mutex_lock(), it will WARN on underflow and fail
+ * to decrement when saturated at REFCOUNT_SATURATED.
+ *
+ * Provides release memory ordering, such that prior loads and stores are done
+ * before, and provides a control dependency such that free() must come after.
+ * See the comment on top.
+ *
+ * Return: true and hold mutex if able to decrement refcount to 0, false
+ *         otherwise
+ */
 extern __must_check bool refcount_dec_and_mutex_lock(refcount_t *r, struct mutex *lock) __cond_acquires(lock);
+
+/**
+ * refcount_dec_and_lock - return holding spinlock if able to decrement
+ *                         refcount to 0
+ * @r: the refcount
+ * @lock: the spinlock to be locked
+ *
+ * Similar to atomic_dec_and_lock(), it will WARN on underflow and fail to
+ * decrement when saturated at REFCOUNT_SATURATED.
+ *
+ * Provides release memory ordering, such that prior loads and stores are done
+ * before, and provides a control dependency such that free() must come after.
+ * See the comment on top.
+ *
+ * Return: true and hold spinlock if able to decrement refcount to 0, false
+ *         otherwise
+ */
 extern __must_check bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock) __cond_acquires(lock);
+
+/**
+ * refcount_dec_and_lock_irqsave - return holding spinlock with disabled
+ *                                 interrupts if able to decrement refcount to 0
+ * @r: the refcount
+ * @lock: the spinlock to be locked
+ * @flags: saved IRQ-flags if the is acquired
+ *
+ * Same as refcount_dec_and_lock() above except that the spinlock is acquired
+ * with disabled interrupts.
+ *
+ * Return: true and hold spinlock if able to decrement refcount to 0, false
+ *         otherwise
+ */
 extern __must_check bool refcount_dec_and_lock_irqsave(refcount_t *r,
 						       spinlock_t *lock,
 						       unsigned long *flags) __cond_acquires(lock);
diff --git a/lib/refcount.c b/lib/refcount.c
index a207a8f22b3c..c37edf66994f 100644
--- a/lib/refcount.c
+++ b/lib/refcount.c
@@ -94,22 +94,6 @@ bool refcount_dec_not_one(refcount_t *r)
 }
 EXPORT_SYMBOL(refcount_dec_not_one);
 
-/**
- * refcount_dec_and_mutex_lock - return holding mutex if able to decrement
- *                               refcount to 0
- * @r: the refcount
- * @lock: the mutex to be locked
- *
- * Similar to atomic_dec_and_mutex_lock(), it will WARN on underflow and fail
- * to decrement when saturated at REFCOUNT_SATURATED.
- *
- * Provides release memory ordering, such that prior loads and stores are done
- * before, and provides a control dependency such that free() must come after.
- * See the comment on top.
- *
- * Return: true and hold mutex if able to decrement refcount to 0, false
- *         otherwise
- */
 bool refcount_dec_and_mutex_lock(refcount_t *r, struct mutex *lock)
 {
 	if (refcount_dec_not_one(r))
@@ -125,22 +109,6 @@ bool refcount_dec_and_mutex_lock(refcount_t *r, struct mutex *lock)
 }
 EXPORT_SYMBOL(refcount_dec_and_mutex_lock);
 
-/**
- * refcount_dec_and_lock - return holding spinlock if able to decrement
- *                         refcount to 0
- * @r: the refcount
- * @lock: the spinlock to be locked
- *
- * Similar to atomic_dec_and_lock(), it will WARN on underflow and fail to
- * decrement when saturated at REFCOUNT_SATURATED.
- *
- * Provides release memory ordering, such that prior loads and stores are done
- * before, and provides a control dependency such that free() must come after.
- * See the comment on top.
- *
- * Return: true and hold spinlock if able to decrement refcount to 0, false
- *         otherwise
- */
 bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock)
 {
 	if (refcount_dec_not_one(r))
@@ -156,19 +124,6 @@ bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock)
 }
 EXPORT_SYMBOL(refcount_dec_and_lock);
 
-/**
- * refcount_dec_and_lock_irqsave - return holding spinlock with disabled
- *                                 interrupts if able to decrement refcount to 0
- * @r: the refcount
- * @lock: the spinlock to be locked
- * @flags: saved IRQ-flags if the is acquired
- *
- * Same as refcount_dec_and_lock() above except that the spinlock is acquired
- * with disabled interrupts.
- *
- * Return: true and hold spinlock if able to decrement refcount to 0, false
- *         otherwise
- */
 bool refcount_dec_and_lock_irqsave(refcount_t *r, spinlock_t *lock,
 				   unsigned long *flags)
 {
-- 
2.39.3

[PATCH 2/3] refcount: introduce __refcount_dec_and_lock macro

[Alexander Aring]

This patch adds the __refcount_dec_and_lock macro to generate code for a
lock specific refcount_dec_and_lock implementation. Existing
refcount_dec_and_lock implementation are updated to use the new
__refcount_dec_and_lock macro. In future other lock implementation can
added to use the refcount_dec_and_lock trick to only hold the lock when
the refcount is going to be zero. Per subsystem own lock implementation
can use the macro as well to provide such implementation for their own
locking type.

Co-developed: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 include/linux/refcount.h | 25 +++++++++++++++++++++++++
 lib/refcount.c           | 36 ++++++------------------------------
 2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/include/linux/refcount.h b/include/linux/refcount.h
index 741cc6295f54..d5cf40d3f4bc 100644
--- a/include/linux/refcount.h
+++ b/include/linux/refcount.h
@@ -362,6 +362,31 @@ static inline void refcount_dec(refcount_t *r)
 extern __must_check bool refcount_dec_if_one(refcount_t *r);
 extern __must_check bool refcount_dec_not_one(refcount_t *r);
 
+/**
+ * __refcount_dec_and_lock - macro to create code to holding a lock if being
+ *                           able to decremnt refcount to 0
+ * @_ref: the refcount
+ * @_lock: lock function call code
+ * @_unlock: unlock function call code
+ *
+ * The result will be directly returned as a right operand operation. Uusally
+ * the caller use it directly after a return statement.
+ */
+#define __refcount_dec_and_lock(_ref, _lock, _unlock)	\
+({							\
+	bool _ret = false;				\
+							\
+	if (!refcount_dec_not_one(_ref)) {		\
+		_lock;					\
+		if (!refcount_dec_and_test(_ref))	\
+			_unlock;			\
+		else					\
+			_ret = true;			\
+	}						\
+							\
+	_ret;						\
+})
+
 /**
  * refcount_dec_and_mutex_lock - return holding mutex if able to decrement
  *                               refcount to 0
diff --git a/lib/refcount.c b/lib/refcount.c
index c37edf66994f..5511498df708 100644
--- a/lib/refcount.c
+++ b/lib/refcount.c
@@ -96,46 +96,22 @@ EXPORT_SYMBOL(refcount_dec_not_one);
 
 bool refcount_dec_and_mutex_lock(refcount_t *r, struct mutex *lock)
 {
-	if (refcount_dec_not_one(r))
-		return false;
-
-	mutex_lock(lock);
-	if (!refcount_dec_and_test(r)) {
-		mutex_unlock(lock);
-		return false;
-	}
-
-	return true;
+	return __refcount_dec_and_lock(r, mutex_lock(lock),
+				       mutex_unlock(lock));
 }
 EXPORT_SYMBOL(refcount_dec_and_mutex_lock);
 
 bool refcount_dec_and_lock(refcount_t *r, spinlock_t *lock)
 {
-	if (refcount_dec_not_one(r))
-		return false;
-
-	spin_lock(lock);
-	if (!refcount_dec_and_test(r)) {
-		spin_unlock(lock);
-		return false;
-	}
-
-	return true;
+	return __refcount_dec_and_lock(r, spin_lock(lock),
+				       spin_unlock(lock));
 }
 EXPORT_SYMBOL(refcount_dec_and_lock);
 
 bool refcount_dec_and_lock_irqsave(refcount_t *r, spinlock_t *lock,
 				   unsigned long *flags)
 {
-	if (refcount_dec_not_one(r))
-		return false;
-
-	spin_lock_irqsave(lock, *flags);
-	if (!refcount_dec_and_test(r)) {
-		spin_unlock_irqrestore(lock, *flags);
-		return false;
-	}
-
-	return true;
+	return __refcount_dec_and_lock(r, spin_lock_irqsave(lock, *flags),
+				       spin_unlock_irqrestore(lock, *flags));
 }
 EXPORT_SYMBOL(refcount_dec_and_lock_irqsave);
-- 
2.39.3

Re: [PATCH 2/3] refcount: introduce __refcount_dec_and_lock macro

[Peter Zijlstra]

On Mon, Nov 06, 2023 at 02:11:37PM -0500, Alexander Aring wrote:
> This patch adds the __refcount_dec_and_lock macro to generate code for a
> lock specific refcount_dec_and_lock implementation. Existing
> refcount_dec_and_lock implementation are updated to use the new
> __refcount_dec_and_lock macro. In future other lock implementation can
> added to use the refcount_dec_and_lock trick to only hold the lock when
> the refcount is going to be zero. Per subsystem own lock implementation
> can use the macro as well to provide such implementation for their own
> locking type.
> 
> Co-developed: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Alexander Aring <aahringo@redhat.com>

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[PATCH 3/3] kref: introduce __kref_put_lock macro

[Alexander Aring]

This patch introduce the __kref_put_lock macro to easily write a
kref_put_lock functionality based on refcount_dec_and_lock functions.
Existing per lock type specific kref_put_lock functionality are updated
to use the new __kref_put_lock macro.

Co-developed: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 include/linux/kref.h | 37 +++++++++++++++++++++++++++----------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/include/linux/kref.h b/include/linux/kref.h
index d32e21a2538c..d8e26ac1d54f 100644
--- a/include/linux/kref.h
+++ b/include/linux/kref.h
@@ -68,26 +68,43 @@ static inline int kref_put(struct kref *kref, void (*release)(struct kref *kref)
 	return 0;
 }
 
+/**
+ * __kref_dec_and_lock - macro to create code to holding a lock and call the
+ *                       release callback if being able to decremnt refcount
+ *                       to 0
+ * @kref: the kref
+ * @_release: callback for the release function
+ * @_ref_dec_and_lock: ref_and_lock lock specific function call code
+ *
+ * The result will be directly returned as a right operand operation. Uusally
+ * the caller use it directly after a return statement.
+ */
+#define __kref_put_lock(_kref, _release, _ref_dec_and_lock)	\
+({								\
+	int _ret = 0;						\
+								\
+	if (_ref_dec_and_lock) {				\
+		_release(kref);					\
+		_ret = 1;					\
+	}							\
+								\
+	_ret;							\
+})
+
 static inline int kref_put_mutex(struct kref *kref,
 				 void (*release)(struct kref *kref),
 				 struct mutex *lock)
 {
-	if (refcount_dec_and_mutex_lock(&kref->refcount, lock)) {
-		release(kref);
-		return 1;
-	}
-	return 0;
+	return __kref_put_lock(kref, release,
+			       refcount_dec_and_mutex_lock(&kref->refcount, lock));
 }
 
 static inline int kref_put_lock(struct kref *kref,
 				void (*release)(struct kref *kref),
 				spinlock_t *lock)
 {
-	if (refcount_dec_and_lock(&kref->refcount, lock)) {
-		release(kref);
-		return 1;
-	}
-	return 0;
+	return __kref_put_lock(kref, release,
+			       refcount_dec_and_lock(&kref->refcount, lock));
 }
 
 /**
-- 
2.39.3

Re: [PATCH 1/3] refcount: move kdoc to header definition

[Christoph Hellwig]

On Mon, Nov 06, 2023 at 02:11:36PM -0500, Alexander Aring wrote:
> This patch moves the kdoc for refcount_dec_and_lock functionality to
> it's header prototype declarations.

That's not how kerneldoc works.  The comments should be next to the
implementation.

Re: [PATCH 1/3] refcount: move kdoc to header definition

[Alexander Aring]

Hi,

On Thu, Nov 9, 2023 at 2:10 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Nov 06, 2023 at 02:11:36PM -0500, Alexander Aring wrote:
> > This patch moves the kdoc for refcount_dec_and_lock functionality to
> > it's header prototype declarations.
>
> That's not how kerneldoc works.  The comments should be next to the
> implementation.
>

ok, saved for next time. Thanks.

I will drop this patch.

- Alex