Skip to content

Commit

Permalink
locking/atomic: scripts: generate kerneldoc comments
Browse files Browse the repository at this point in the history
Currently the atomics are documented in Documentation/atomic_t.txt, and
have no kerneldoc comments. There are a sufficient number of gotchas
(e.g. semantics, noinstr-safety) that it would be nice to have comments
to call these out, and it would be nice to have kerneldoc comments such
that these can be collated.

While it's possible to derive the semantics from the code, this can be
painful given the amount of indirection we currently have (e.g. fallback
paths), and it's easy to be mislead by naming, e.g.

* The unconditional void-returning ops *only* have relaxed variants
  without a _relaxed suffix, and can easily be mistaken for being fully
  ordered.

  It would be nice to give these a _relaxed() suffix, but this would
  result in significant churn throughout the kernel.

* Our naming of conditional and unconditional+test ops is rather
  inconsistent, and it can be difficult to derive the name of an
  operation, or to identify where an op is conditional or
  unconditional+test.

  Some ops are clearly conditional:
  - dec_if_positive
  - add_unless
  - dec_unless_positive
  - inc_unless_negative

  Some ops are clearly unconditional+test:
  - sub_and_test
  - dec_and_test
  - inc_and_test

  However, what exactly those test is not obvious. A _test_zero suffix
  might be clearer.

  Others could be read ambiguously:
  - inc_not_zero	// conditional
  - add_negative	// unconditional+test

  It would probably be worth renaming these, e.g. to inc_unless_zero and
  add_test_negative.

As a step towards making this more consistent and easier to understand,
this patch adds kerneldoc comments for all generated *atomic*_*()
functions. These are generated from templates, with some common text
shared, making it easy to extend these in future if necessary.

I've tried to make these as consistent and clear as possible, and I've
deliberately ensured:

* All ops have their ordering explicitly mentioned in the short and long
  description.

* All test ops have "test" in their short description.

* All ops are described as an expression using their usual C operator.
  For example:

  andnot: "Atomically updates @v to (@v & ~@i)"
  inc:    "Atomically updates @v to (@v + 1)"

  Which may be clearer to non-naative English speakers, and allows all
  the operations to be described in the same style.

* All conditional ops have their condition described as an expression
  using the usual C operators. For example:

  add_unless: "If (@v != @U), atomically updates @v to (@v + @i)"
  cmpxchg:    "If (@v == @old), atomically updates @v to @new"

  Which may be clearer to non-naative English speakers, and allows all
  the operations to be described in the same style.

* All bitwise ops (and,andnot,or,xor) explicitly mention that they are
  bitwise in their short description, so that they are not mistaken for
  performing their logical equivalents.

* The noinstr safety of each op is explicitly described, with a
  description of whether or not to use the raw_ form of the op.

There should be no functional change as a result of this patch.

Reported-by: Paul E. McKenney <[email protected]>
Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
  • Loading branch information
Mark Rutland authored and Peter Zijlstra committed Jun 5, 2023
1 parent 8aaf297 commit ad81107
Show file tree
Hide file tree
Showing 29 changed files with 5,940 additions and 7 deletions.
1,848 changes: 1,847 additions & 1 deletion include/linux/atomic/atomic-arch-fallback.h

Large diffs are not rendered by default.

2,771 changes: 2,770 additions & 1 deletion include/linux/atomic/atomic-instrumented.h

Large diffs are not rendered by default.

925 changes: 924 additions & 1 deletion include/linux/atomic/atomic-long.h

Large diffs are not rendered by default.

112 changes: 108 additions & 4 deletions scripts/atomic/atomic-tbl.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,16 @@ meta_has_relaxed()
meta_in "$1" "BFIR"
}

#find_fallback_template(pfx, name, sfx, order)
find_fallback_template()
#meta_is_implicitly_relaxed(meta)
meta_is_implicitly_relaxed()
{
meta_in "$1" "vls"
}

#find_template(tmpltype, pfx, name, sfx, order)
find_template()
{
local tmpltype="$1"; shift
local pfx="$1"; shift
local name="$1"; shift
local sfx="$1"; shift
Expand All @@ -52,8 +59,8 @@ find_fallback_template()
#
# Start at the most specific, and fall back to the most general. Once
# we find a specific fallback, don't bother looking for more.
for base in "${pfx}${name}${sfx}${order}" "${name}"; do
file="${ATOMICDIR}/fallbacks/${base}"
for base in "${pfx}${name}${sfx}${order}" "${pfx}${name}${sfx}" "${name}"; do
file="${ATOMICDIR}/${tmpltype}/${base}"

if [ -f "${file}" ]; then
printf "${file}"
Expand All @@ -62,6 +69,18 @@ find_fallback_template()
done
}

#find_fallback_template(pfx, name, sfx, order)
find_fallback_template()
{
find_template "fallbacks" "$@"
}

#find_kerneldoc_template(pfx, name, sfx, order)
find_kerneldoc_template()
{
find_template "kerneldoc" "$@"
}

#gen_ret_type(meta, int)
gen_ret_type() {
local meta="$1"; shift
Expand Down Expand Up @@ -142,6 +161,91 @@ gen_args()
done
}

#gen_desc_return(meta)
gen_desc_return()
{
local meta="$1"; shift

case "${meta}" in
[v])
printf "Return: Nothing."
;;
[Ff])
printf "Return: The original value of @v."
;;
[R])
printf "Return: The updated value of @v."
;;
[l])
printf "Return: The value of @v."
;;
esac
}

#gen_template_kerneldoc(template, class, meta, pfx, name, sfx, order, atomic, int, args...)
gen_template_kerneldoc()
{
local template="$1"; shift
local class="$1"; shift
local meta="$1"; shift
local pfx="$1"; shift
local name="$1"; shift
local sfx="$1"; shift
local order="$1"; shift
local atomic="$1"; shift
local int="$1"; shift

local atomicname="${atomic}_${pfx}${name}${sfx}${order}"

local ret="$(gen_ret_type "${meta}" "${int}")"
local retstmt="$(gen_ret_stmt "${meta}")"
local params="$(gen_params "${int}" "${atomic}" "$@")"
local args="$(gen_args "$@")"
local desc_order=""
local desc_instrumentation=""
local desc_return=""

if [ ! -z "${order}" ]; then
desc_order="${order##_}"
elif meta_is_implicitly_relaxed "${meta}"; then
desc_order="relaxed"
else
desc_order="full"
fi

if [ -z "${class}" ]; then
desc_noinstr="Unsafe to use in noinstr code; use raw_${atomicname}() there."
else
desc_noinstr="Safe to use in noinstr code; prefer ${atomicname}() elsewhere."
fi

desc_return="$(gen_desc_return "${meta}")"

. ${template}
}

#gen_kerneldoc(class, meta, pfx, name, sfx, order, atomic, int, args...)
gen_kerneldoc()
{
local class="$1"; shift
local meta="$1"; shift
local pfx="$1"; shift
local name="$1"; shift
local sfx="$1"; shift
local order="$1"; shift

local atomicname="${atomic}_${pfx}${name}${sfx}${order}"

local tmpl="$(find_kerneldoc_template "${pfx}" "${name}" "${sfx}" "${order}")"
if [ -z "${tmpl}" ]; then
printf "/*\n"
printf " * No kerneldoc available for ${class}${atomicname}\n"
printf " */\n"
else
gen_template_kerneldoc "${tmpl}" "${class}" "${meta}" "${pfx}" "${name}" "${sfx}" "${order}" "$@"
fi
}

#gen_proto_order_variants(meta, pfx, name, sfx, ...)
gen_proto_order_variants()
{
Expand Down
2 changes: 2 additions & 0 deletions scripts/atomic/gen-atomic-fallback.sh
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ gen_proto_order_variant()
local params="$(gen_params "${int}" "${atomic}" "$@")"
local args="$(gen_args "$@")"

gen_kerneldoc "raw_" "${meta}" "${pfx}" "${name}" "${sfx}" "${order}" "${atomic}" "${int}" "$@"

printf "static __always_inline ${ret}\n"
printf "raw_${atomicname}(${params})\n"
printf "{\n"
Expand Down
2 changes: 2 additions & 0 deletions scripts/atomic/gen-atomic-instrumented.sh
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ gen_proto_order_variant()
local args="$(gen_args "$@")"
local retstmt="$(gen_ret_stmt "${meta}")"

gen_kerneldoc "" "${meta}" "${pfx}" "${name}" "${sfx}" "${order}" "${atomic}" "${int}" "$@"

cat <<EOF
static __always_inline ${ret}
${atomicname}(${params})
Expand Down
2 changes: 2 additions & 0 deletions scripts/atomic/gen-atomic-long.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ gen_proto_order_variant()
local argscast_64="$(gen_args_cast "s64" "atomic64" "$@")"
local retstmt="$(gen_ret_stmt "${meta}")"

gen_kerneldoc "raw_" "${meta}" "${pfx}" "${name}" "${sfx}" "${order}" "atomic_long" "long" "$@"

cat <<EOF
static __always_inline ${ret}
raw_atomic_long_${atomicname}(${params})
Expand Down
13 changes: 13 additions & 0 deletions scripts/atomic/kerneldoc/add
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic add with ${desc_order} ordering
* @i: ${int} value to add
* @v: pointer to ${atomic}_t
*
* Atomically updates @v to (@v + @i) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* ${desc_return}
*/
EOF
13 changes: 13 additions & 0 deletions scripts/atomic/kerneldoc/add_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic add and test if negative with ${desc_order} ordering
* @i: ${int} value to add
* @v: pointer to ${atomic}_t
*
* Atomically updates @v to (@v + @i) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* Return: @true if the resulting value of @v is negative, @false otherwise.
*/
EOF
18 changes: 18 additions & 0 deletions scripts/atomic/kerneldoc/add_unless
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
if [ -z "${pfx}" ]; then
desc_return="Return: @true if @v was updated, @false otherwise."
fi

cat <<EOF
/**
* ${class}${atomicname}() - atomic add unless value with ${desc_order} ordering
* @v: pointer to ${atomic}_t
* @a: ${int} value to add
* @u: ${int} value to compare with
*
* If (@v != @u), atomically updates @v to (@v + @a) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* ${desc_return}
*/
EOF
13 changes: 13 additions & 0 deletions scripts/atomic/kerneldoc/and
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic bitwise AND with ${desc_order} ordering
* @i: ${int} value
* @v: pointer to ${atomic}_t
*
* Atomically updates @v to (@v & @i) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* ${desc_return}
*/
EOF
13 changes: 13 additions & 0 deletions scripts/atomic/kerneldoc/andnot
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic bitwise AND NOT with ${desc_order} ordering
* @i: ${int} value
* @v: pointer to ${atomic}_t
*
* Atomically updates @v to (@v & ~@i) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* ${desc_return}
*/
EOF
14 changes: 14 additions & 0 deletions scripts/atomic/kerneldoc/cmpxchg
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic compare and exchange with ${desc_order} ordering
* @v: pointer to ${atomic}_t
* @old: ${int} value to compare with
* @new: ${int} value to assign
*
* If (@v == @old), atomically updates @v to @new with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* Return: The original value of @v.
*/
EOF
12 changes: 12 additions & 0 deletions scripts/atomic/kerneldoc/dec
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic decrement with ${desc_order} ordering
* @v: pointer to ${atomic}_t
*
* Atomically updates @v to (@v - 1) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* ${desc_return}
*/
EOF
12 changes: 12 additions & 0 deletions scripts/atomic/kerneldoc/dec_and_test
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic decrement and test if zero with ${desc_order} ordering
* @v: pointer to ${atomic}_t
*
* Atomically updates @v to (@v - 1) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* Return: @true if the resulting value of @v is zero, @false otherwise.
*/
EOF
12 changes: 12 additions & 0 deletions scripts/atomic/kerneldoc/dec_if_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic decrement if positive with ${desc_order} ordering
* @v: pointer to ${atomic}_t
*
* If (@v > 0), atomically updates @v to (@v - 1) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* Return: @true if @v was updated, @false otherwise.
*/
EOF
12 changes: 12 additions & 0 deletions scripts/atomic/kerneldoc/dec_unless_positive
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic decrement unless positive with ${desc_order} ordering
* @v: pointer to ${atomic}_t
*
* If (@v <= 0), atomically updates @v to (@v - 1) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* Return: @true if @v was updated, @false otherwise.
*/
EOF
12 changes: 12 additions & 0 deletions scripts/atomic/kerneldoc/inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic increment with ${desc_order} ordering
* @v: pointer to ${atomic}_t
*
* Atomically updates @v to (@v + 1) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* ${desc_return}
*/
EOF
12 changes: 12 additions & 0 deletions scripts/atomic/kerneldoc/inc_and_test
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic increment and test if zero with ${desc_order} ordering
* @v: pointer to ${atomic}_t
*
* Atomically updates @v to (@v + 1) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* Return: @true if the resulting value of @v is zero, @false otherwise.
*/
EOF
12 changes: 12 additions & 0 deletions scripts/atomic/kerneldoc/inc_not_zero
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic increment unless zero with ${desc_order} ordering
* @v: pointer to ${atomic}_t
*
* If (@v != 0), atomically updates @v to (@v + 1) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* Return: @true if @v was updated, @false otherwise.
*/
EOF
12 changes: 12 additions & 0 deletions scripts/atomic/kerneldoc/inc_unless_negative
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic increment unless negative with ${desc_order} ordering
* @v: pointer to ${atomic}_t
*
* If (@v >= 0), atomically updates @v to (@v + 1) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* Return: @true if @v was updated, @false otherwise.
*/
EOF
13 changes: 13 additions & 0 deletions scripts/atomic/kerneldoc/or
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic bitwise OR with ${desc_order} ordering
* @i: ${int} value
* @v: pointer to ${atomic}_t
*
* Atomically updates @v to (@v | @i) with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* ${desc_return}
*/
EOF
12 changes: 12 additions & 0 deletions scripts/atomic/kerneldoc/read
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
cat <<EOF
/**
* ${class}${atomicname}() - atomic load with ${desc_order} ordering
* @v: pointer to ${atomic}_t
*
* Atomically loads the value of @v with ${desc_order} ordering.
*
* ${desc_noinstr}
*
* Return: The value loaded from @v.
*/
EOF
Loading

0 comments on commit ad81107

Please sign in to comment.