diff mbox series

RFC: Checkpatch: Introduce list of functions that need memory barriers.

Message ID 20250104211554.20205-1-manfred@colorfullife.com (mailing list archive)
State New
Headers show
Series RFC: Checkpatch: Introduce list of functions that need memory barriers. | expand

Commit Message

Manfred Spraul Jan. 4, 2025, 9:15 p.m. UTC
(depending on output of the discussion in
https://lore.kernel.org/lkml/20250102163320.GA17691@redhat.com/T/#u

It does not make sense to change it now, but I do not see a reason
to single out waitqueue_active().
The code is copy&paste, it seems to work.
There is already a recommendation for spin_is_locked()
)


2nd spinoff from the fs/pipe discussion, and ortogonal to both
the initial topic (use waitqueue_active()) and the 2nd topic
(do not wake up writers if the pipe is not writable)

Memory barriers must be paired to be effective, thus it is
mandatory to add comments that explain the pairing.
Several functions depend on the caller to take care of this.

There is already a request to add a comment for waitqueue_active(),
but there are further comparable functions:

 wq_has_sleepers(): No barrier is needed if the function
  is paired with prepare_to_wait(). With add_wait_queue(),
  a barrier is needed after the add_wait_queue() call.

- spin_is_locked(): the ACQUIRE barrier from spin_lock()
  is on the load, not on the store. Thus spin_is_locked()
  may return false even though the lock is already taken.
  Avoid to use it outside of debug code.

(and, for completeness)
- waitqueue_active(): Usually, a memory barrier before
  the call is needed, and if add_wait_queue() is used, also
  a barrier after add_wait_queue. See wait.h for details.

-
Signed-off-by: Manfred Spraul <manfred@colorfullife.com>
---
 scripts/checkpatch.pl | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9eed3683ad76..8bf5849ee108 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6685,11 +6685,17 @@  sub process {
 			     "__smp memory barriers shouldn't be used outside barrier.h and asm-generic\n" . $herecurr);
 		}
 
-# check for waitqueue_active without a comment.
-		if ($line =~ /\bwaitqueue_active\s*\(/) {
+# check for functions that are only safe with memory barriers without a comment.
+		my $need_barriers = qr{
+			waitqueue_active|
+			wq_has_sleeper|
+			spin_is_locked
+		}x;
+
+		if ($line =~ /\b(?:$need_barriers)\s*\(/) {
 			if (!ctx_has_comment($first_line, $linenr)) {
-				WARN("WAITQUEUE_ACTIVE",
-				     "waitqueue_active without comment\n" . $herecurr);
+				WARN("NEED_MEMORY_BARRIERS",
+				     "function that usually depend on manual memory barriers without comment\n" . $herecurr);
 			}
 		}