The ${atomic}_dec_if_positive() ops are unlike all the other conditional
atomic ops. Rather than returning a boolean success value, these return
the value that the atomic variable would be updated to, even when no
update is performed.
We missed this when adding kerneldoc comments, and the documentation for
${atomic}_dec_if_positive() erroneously states:
| Return: @true if @v was updated, @false otherwise.
Ideally we'd clean this up by aligning ${atomic}_dec_if_positive() with
the usual atomic op conventions: with ${atomic}_fetch_dec_if_positive()
for those who care about the value of the varaible, and
${atomic}_dec_if_positive() returning a boolean success value.
In the mean time, align the documentation with the current reality.
Fixes: ad8110706f381170 ("locking/atomic: scripts: generate kerneldoc comments")
Signed-off-by: Mark Rutland <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Will Deacon <[email protected]>
---
include/linux/atomic/atomic-arch-fallback.h | 6 +++---
include/linux/atomic/atomic-instrumented.h | 8 ++++----
include/linux/atomic/atomic-long.h | 4 ++--
scripts/atomic/kerneldoc/dec_if_positive | 2 +-
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/linux/atomic/atomic-arch-fallback.h b/include/linux/atomic/atomic-arch-fallback.h
index 8cded57dd7a6f..18f5744dfb5d8 100644
--- a/include/linux/atomic/atomic-arch-fallback.h
+++ b/include/linux/atomic/atomic-arch-fallback.h
@@ -2520,7 +2520,7 @@ raw_atomic_dec_unless_positive(atomic_t *v)
*
* Safe to use in noinstr code; prefer atomic_dec_if_positive() elsewhere.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline int
raw_atomic_dec_if_positive(atomic_t *v)
@@ -4636,7 +4636,7 @@ raw_atomic64_dec_unless_positive(atomic64_t *v)
*
* Safe to use in noinstr code; prefer atomic64_dec_if_positive() elsewhere.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline s64
raw_atomic64_dec_if_positive(atomic64_t *v)
@@ -4657,4 +4657,4 @@ raw_atomic64_dec_if_positive(atomic64_t *v)
}
#endif /* _LINUX_ATOMIC_FALLBACK_H */
-// 3916f02c038baa3f5190d275f68b9211667fcc9d
+// 202b45c7db600ce36198eb1f1fc2c2d5268ace2d
diff --git a/include/linux/atomic/atomic-instrumented.h b/include/linux/atomic/atomic-instrumented.h
index ebfc795f921b9..d401b406ef7c4 100644
--- a/include/linux/atomic/atomic-instrumented.h
+++ b/include/linux/atomic/atomic-instrumented.h
@@ -1570,7 +1570,7 @@ atomic_dec_unless_positive(atomic_t *v)
*
* Unsafe to use in noinstr code; use raw_atomic_dec_if_positive() there.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline int
atomic_dec_if_positive(atomic_t *v)
@@ -3134,7 +3134,7 @@ atomic64_dec_unless_positive(atomic64_t *v)
*
* Unsafe to use in noinstr code; use raw_atomic64_dec_if_positive() there.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline s64
atomic64_dec_if_positive(atomic64_t *v)
@@ -4698,7 +4698,7 @@ atomic_long_dec_unless_positive(atomic_long_t *v)
*
* Unsafe to use in noinstr code; use raw_atomic_long_dec_if_positive() there.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline long
atomic_long_dec_if_positive(atomic_long_t *v)
@@ -5000,4 +5000,4 @@ atomic_long_dec_if_positive(atomic_long_t *v)
#endif /* _LINUX_ATOMIC_INSTRUMENTED_H */
-// 06cec02e676a484857aee38b0071a1d846ec9457
+// 1568f875fef72097413caab8339120c065a39aa4
diff --git a/include/linux/atomic/atomic-long.h b/include/linux/atomic/atomic-long.h
index f6df2adadf997..c82947170ddc8 100644
--- a/include/linux/atomic/atomic-long.h
+++ b/include/linux/atomic/atomic-long.h
@@ -1782,7 +1782,7 @@ raw_atomic_long_dec_unless_positive(atomic_long_t *v)
*
* Safe to use in noinstr code; prefer atomic_long_dec_if_positive() elsewhere.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline long
raw_atomic_long_dec_if_positive(atomic_long_t *v)
@@ -1795,4 +1795,4 @@ raw_atomic_long_dec_if_positive(atomic_long_t *v)
}
#endif /* _LINUX_ATOMIC_LONG_H */
-// 029d2e3a493086671e874a4c2e0e42084be42403
+// 4ef23f98c73cff96d239896175fd26b10b88899e
diff --git a/scripts/atomic/kerneldoc/dec_if_positive b/scripts/atomic/kerneldoc/dec_if_positive
index 7c742866fb6b6..04f1aed3cf830 100644
--- a/scripts/atomic/kerneldoc/dec_if_positive
+++ b/scripts/atomic/kerneldoc/dec_if_positive
@@ -7,6 +7,6 @@ cat <<EOF
*
* ${desc_noinstr}
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
EOF
--
2.30.2
On Thu, Jun 15, 2023 at 02:27:34PM +0100, Mark Rutland wrote:
> The ${atomic}_dec_if_positive() ops are unlike all the other conditional
> atomic ops. Rather than returning a boolean success value, these return
> the value that the atomic variable would be updated to, even when no
> update is performed.
>
> We missed this when adding kerneldoc comments, and the documentation for
> ${atomic}_dec_if_positive() erroneously states:
>
> | Return: @true if @v was updated, @false otherwise.
>
> Ideally we'd clean this up by aligning ${atomic}_dec_if_positive() with
> the usual atomic op conventions: with ${atomic}_fetch_dec_if_positive()
> for those who care about the value of the varaible, and
> ${atomic}_dec_if_positive() returning a boolean success value.
>
> In the mean time, align the documentation with the current reality.
>
> Fixes: ad8110706f381170 ("locking/atomic: scripts: generate kerneldoc comments")
> Signed-off-by: Mark Rutland <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Kees Cook <[email protected]>
> Cc: Paul E. McKenney <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Will Deacon <[email protected]>
Reviewed-by: Paul E. McKenney <[email protected]>
> ---
> include/linux/atomic/atomic-arch-fallback.h | 6 +++---
> include/linux/atomic/atomic-instrumented.h | 8 ++++----
> include/linux/atomic/atomic-long.h | 4 ++--
> scripts/atomic/kerneldoc/dec_if_positive | 2 +-
> 4 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/atomic/atomic-arch-fallback.h b/include/linux/atomic/atomic-arch-fallback.h
> index 8cded57dd7a6f..18f5744dfb5d8 100644
> --- a/include/linux/atomic/atomic-arch-fallback.h
> +++ b/include/linux/atomic/atomic-arch-fallback.h
> @@ -2520,7 +2520,7 @@ raw_atomic_dec_unless_positive(atomic_t *v)
> *
> * Safe to use in noinstr code; prefer atomic_dec_if_positive() elsewhere.
> *
> - * Return: @true if @v was updated, @false otherwise.
> + * Return: The old value of (@v - 1), regardless of whether @v was updated.
> */
> static __always_inline int
> raw_atomic_dec_if_positive(atomic_t *v)
> @@ -4636,7 +4636,7 @@ raw_atomic64_dec_unless_positive(atomic64_t *v)
> *
> * Safe to use in noinstr code; prefer atomic64_dec_if_positive() elsewhere.
> *
> - * Return: @true if @v was updated, @false otherwise.
> + * Return: The old value of (@v - 1), regardless of whether @v was updated.
> */
> static __always_inline s64
> raw_atomic64_dec_if_positive(atomic64_t *v)
> @@ -4657,4 +4657,4 @@ raw_atomic64_dec_if_positive(atomic64_t *v)
> }
>
> #endif /* _LINUX_ATOMIC_FALLBACK_H */
> -// 3916f02c038baa3f5190d275f68b9211667fcc9d
> +// 202b45c7db600ce36198eb1f1fc2c2d5268ace2d
> diff --git a/include/linux/atomic/atomic-instrumented.h b/include/linux/atomic/atomic-instrumented.h
> index ebfc795f921b9..d401b406ef7c4 100644
> --- a/include/linux/atomic/atomic-instrumented.h
> +++ b/include/linux/atomic/atomic-instrumented.h
> @@ -1570,7 +1570,7 @@ atomic_dec_unless_positive(atomic_t *v)
> *
> * Unsafe to use in noinstr code; use raw_atomic_dec_if_positive() there.
> *
> - * Return: @true if @v was updated, @false otherwise.
> + * Return: The old value of (@v - 1), regardless of whether @v was updated.
> */
> static __always_inline int
> atomic_dec_if_positive(atomic_t *v)
> @@ -3134,7 +3134,7 @@ atomic64_dec_unless_positive(atomic64_t *v)
> *
> * Unsafe to use in noinstr code; use raw_atomic64_dec_if_positive() there.
> *
> - * Return: @true if @v was updated, @false otherwise.
> + * Return: The old value of (@v - 1), regardless of whether @v was updated.
> */
> static __always_inline s64
> atomic64_dec_if_positive(atomic64_t *v)
> @@ -4698,7 +4698,7 @@ atomic_long_dec_unless_positive(atomic_long_t *v)
> *
> * Unsafe to use in noinstr code; use raw_atomic_long_dec_if_positive() there.
> *
> - * Return: @true if @v was updated, @false otherwise.
> + * Return: The old value of (@v - 1), regardless of whether @v was updated.
> */
> static __always_inline long
> atomic_long_dec_if_positive(atomic_long_t *v)
> @@ -5000,4 +5000,4 @@ atomic_long_dec_if_positive(atomic_long_t *v)
>
>
> #endif /* _LINUX_ATOMIC_INSTRUMENTED_H */
> -// 06cec02e676a484857aee38b0071a1d846ec9457
> +// 1568f875fef72097413caab8339120c065a39aa4
> diff --git a/include/linux/atomic/atomic-long.h b/include/linux/atomic/atomic-long.h
> index f6df2adadf997..c82947170ddc8 100644
> --- a/include/linux/atomic/atomic-long.h
> +++ b/include/linux/atomic/atomic-long.h
> @@ -1782,7 +1782,7 @@ raw_atomic_long_dec_unless_positive(atomic_long_t *v)
> *
> * Safe to use in noinstr code; prefer atomic_long_dec_if_positive() elsewhere.
> *
> - * Return: @true if @v was updated, @false otherwise.
> + * Return: The old value of (@v - 1), regardless of whether @v was updated.
> */
> static __always_inline long
> raw_atomic_long_dec_if_positive(atomic_long_t *v)
> @@ -1795,4 +1795,4 @@ raw_atomic_long_dec_if_positive(atomic_long_t *v)
> }
>
> #endif /* _LINUX_ATOMIC_LONG_H */
> -// 029d2e3a493086671e874a4c2e0e42084be42403
> +// 4ef23f98c73cff96d239896175fd26b10b88899e
> diff --git a/scripts/atomic/kerneldoc/dec_if_positive b/scripts/atomic/kerneldoc/dec_if_positive
> index 7c742866fb6b6..04f1aed3cf830 100644
> --- a/scripts/atomic/kerneldoc/dec_if_positive
> +++ b/scripts/atomic/kerneldoc/dec_if_positive
> @@ -7,6 +7,6 @@ cat <<EOF
> *
> * ${desc_noinstr}
> *
> - * Return: @true if @v was updated, @false otherwise.
> + * Return: The old value of (@v - 1), regardless of whether @v was updated.
> */
> EOF
> --
> 2.30.2
>
The following commit has been merged into the locking/core branch of tip:
Commit-ID: b33eb50a92b0a298fa8a6ac350e741c3ec100f6d
Gitweb: https://git.kernel.org/tip/b33eb50a92b0a298fa8a6ac350e741c3ec100f6d
Author: Mark Rutland <[email protected]>
AuthorDate: Thu, 15 Jun 2023 14:27:34 +01:00
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Fri, 16 Jun 2023 16:46:30 +02:00
locking/atomic: scripts: fix ${atomic}_dec_if_positive() kerneldoc
The ${atomic}_dec_if_positive() ops are unlike all the other conditional
atomic ops. Rather than returning a boolean success value, these return
the value that the atomic variable would be updated to, even when no
update is performed.
We missed this when adding kerneldoc comments, and the documentation for
${atomic}_dec_if_positive() erroneously states:
| Return: @true if @v was updated, @false otherwise.
Ideally we'd clean this up by aligning ${atomic}_dec_if_positive() with
the usual atomic op conventions: with ${atomic}_fetch_dec_if_positive()
for those who care about the value of the varaible, and
${atomic}_dec_if_positive() returning a boolean success value.
In the mean time, align the documentation with the current reality.
Fixes: ad8110706f381170 ("locking/atomic: scripts: generate kerneldoc comments")
Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Paul E. McKenney <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
include/linux/atomic/atomic-arch-fallback.h | 6 +++---
include/linux/atomic/atomic-instrumented.h | 8 ++++----
include/linux/atomic/atomic-long.h | 4 ++--
scripts/atomic/kerneldoc/dec_if_positive | 2 +-
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/linux/atomic/atomic-arch-fallback.h b/include/linux/atomic/atomic-arch-fallback.h
index 8cded57..18f5744 100644
--- a/include/linux/atomic/atomic-arch-fallback.h
+++ b/include/linux/atomic/atomic-arch-fallback.h
@@ -2520,7 +2520,7 @@ raw_atomic_dec_unless_positive(atomic_t *v)
*
* Safe to use in noinstr code; prefer atomic_dec_if_positive() elsewhere.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline int
raw_atomic_dec_if_positive(atomic_t *v)
@@ -4636,7 +4636,7 @@ raw_atomic64_dec_unless_positive(atomic64_t *v)
*
* Safe to use in noinstr code; prefer atomic64_dec_if_positive() elsewhere.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline s64
raw_atomic64_dec_if_positive(atomic64_t *v)
@@ -4657,4 +4657,4 @@ raw_atomic64_dec_if_positive(atomic64_t *v)
}
#endif /* _LINUX_ATOMIC_FALLBACK_H */
-// 3916f02c038baa3f5190d275f68b9211667fcc9d
+// 202b45c7db600ce36198eb1f1fc2c2d5268ace2d
diff --git a/include/linux/atomic/atomic-instrumented.h b/include/linux/atomic/atomic-instrumented.h
index ebfc795..d401b40 100644
--- a/include/linux/atomic/atomic-instrumented.h
+++ b/include/linux/atomic/atomic-instrumented.h
@@ -1570,7 +1570,7 @@ atomic_dec_unless_positive(atomic_t *v)
*
* Unsafe to use in noinstr code; use raw_atomic_dec_if_positive() there.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline int
atomic_dec_if_positive(atomic_t *v)
@@ -3134,7 +3134,7 @@ atomic64_dec_unless_positive(atomic64_t *v)
*
* Unsafe to use in noinstr code; use raw_atomic64_dec_if_positive() there.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline s64
atomic64_dec_if_positive(atomic64_t *v)
@@ -4698,7 +4698,7 @@ atomic_long_dec_unless_positive(atomic_long_t *v)
*
* Unsafe to use in noinstr code; use raw_atomic_long_dec_if_positive() there.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline long
atomic_long_dec_if_positive(atomic_long_t *v)
@@ -5000,4 +5000,4 @@ atomic_long_dec_if_positive(atomic_long_t *v)
#endif /* _LINUX_ATOMIC_INSTRUMENTED_H */
-// 06cec02e676a484857aee38b0071a1d846ec9457
+// 1568f875fef72097413caab8339120c065a39aa4
diff --git a/include/linux/atomic/atomic-long.h b/include/linux/atomic/atomic-long.h
index f6df2ad..c829471 100644
--- a/include/linux/atomic/atomic-long.h
+++ b/include/linux/atomic/atomic-long.h
@@ -1782,7 +1782,7 @@ raw_atomic_long_dec_unless_positive(atomic_long_t *v)
*
* Safe to use in noinstr code; prefer atomic_long_dec_if_positive() elsewhere.
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
static __always_inline long
raw_atomic_long_dec_if_positive(atomic_long_t *v)
@@ -1795,4 +1795,4 @@ raw_atomic_long_dec_if_positive(atomic_long_t *v)
}
#endif /* _LINUX_ATOMIC_LONG_H */
-// 029d2e3a493086671e874a4c2e0e42084be42403
+// 4ef23f98c73cff96d239896175fd26b10b88899e
diff --git a/scripts/atomic/kerneldoc/dec_if_positive b/scripts/atomic/kerneldoc/dec_if_positive
index 7c74286..04f1aed 100644
--- a/scripts/atomic/kerneldoc/dec_if_positive
+++ b/scripts/atomic/kerneldoc/dec_if_positive
@@ -7,6 +7,6 @@ cat <<EOF
*
* ${desc_noinstr}
*
- * Return: @true if @v was updated, @false otherwise.
+ * Return: The old value of (@v - 1), regardless of whether @v was updated.
*/
EOF