mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Manfred Spraul <manfred@colorfullife.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Joe Perches <joe@perches.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	WangYuli <wangyuli@uniontech.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Christian Brauner <brauner@kernel.org>,
	1vier1@web.de, Manfred Spraul <manfred@colorfullife.com>
Subject: RFC: Checkpatch: Introduce list of functions that need memory barriers.
Date: Sat,  4 Jan 2025 22:15:54 +0100	[thread overview]
Message-ID: <20250104211554.20205-1-manfred@colorfullife.com> (raw)
In-Reply-To: <20241230153844.GA15134@redhat.com>

(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 --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);
 			}
 		}
 
-- 
2.47.1


      parent reply	other threads:[~2025-01-04 21:16 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-25  9:42 [RESEND PATCH] fs/pipe: Introduce a check to skip sleeping processes during pipe read/write WangYuli
2024-12-25 13:30 ` Andy Shevchenko
2024-12-25 13:53   ` Kent Overstreet
2024-12-25 16:04     ` Mateusz Guzik
2024-12-25 16:32       ` Kent Overstreet
2024-12-25 17:22         ` Mateusz Guzik
2024-12-25 17:41           ` Kent Overstreet
2024-12-25 15:42   ` WangYuli
2024-12-25 16:00     ` Willy Tarreau
2024-12-25 16:32       ` WangYuli
2024-12-25 16:56         ` Willy Tarreau
2024-12-26 16:00 ` Oleg Nesterov
2024-12-26 19:02 ` Linus Torvalds
2024-12-26 20:11   ` Oleg Nesterov
2024-12-26 20:29     ` Linus Torvalds
2024-12-26 20:57       ` Oleg Nesterov
2024-12-27 15:54         ` Oleg Nesterov
2024-12-27 16:43           ` Oleg Nesterov
2024-12-27 18:39     ` Manfred Spraul
2024-12-28 14:32       ` Oleg Nesterov
2024-12-28 15:22         ` Oleg Nesterov
2024-12-28 16:32           ` Oleg Nesterov
2024-12-28 18:53             ` Manfred Spraul
2024-12-29 11:54               ` Oleg Nesterov
2024-12-28 16:45           ` Manfred Spraul
2024-12-29 11:57             ` Oleg Nesterov
2024-12-29 12:41               ` Manfred Spraul
2024-12-29 13:05                 ` Oleg Nesterov
2024-12-29 13:13                   ` Oleg Nesterov
2024-12-29 19:54                     ` Manfred Spraul
2024-12-30 15:38                       ` Oleg Nesterov
2024-12-31 11:14                         ` Manfred Spraul
2024-12-31 19:38                           ` Linus Torvalds
2024-12-31 20:24                             ` Oleg Nesterov
2024-12-31 22:31                               ` Linus Torvalds
2025-01-02 13:57                                 ` Oleg Nesterov
2025-01-04 21:15                         ` Manfred Spraul [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250104211554.20205-1-manfred@colorfullife.com \
    --to=manfred@colorfullife.com \
    --cc=1vier1@web.de \
    --cc=brauner@kernel.org \
    --cc=joe@perches.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=wangyuli@uniontech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®