public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Mateusz Guzik <mjguzik@gmail.com>
To: torvalds@linux-foundation.org
Cc: oleg@redhat.com, brauner@kernel.org, mingo@redhat.com,
	peterz@infradead.org, rostedt@goodmis.org,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Mateusz Guzik <mjguzik@gmail.com>
Subject: [PATCH 3/3] wait: avoid spurious calls to prepare_to_wait_event() in ___wait_event()
Date: Tue,  4 Mar 2025 00:04:09 +0100	[thread overview]
Message-ID: <20250303230409.452687-4-mjguzik@gmail.com> (raw)
In-Reply-To: <20250303230409.452687-1-mjguzik@gmail.com>

In vast majority of cases the condition determining whether the thread
can proceed is true after the first wake up.

However, even in that case the thread ends up calling into
prepare_to_wait_event() again, suffering a spurious irq + lock trip.

Then it calls into finish_wait() to unlink itself.

Note that in case of a pending signal the work done by
prepare_to_wait_event() gets ignored even without the change.

pre-check the condition after waking up instead.

Stats gathared during a kernel build:
bpftrace -e 'kprobe:prepare_to_wait_event,kprobe:finish_wait \
		 { @[probe] = count(); }'

@[kprobe:finish_wait]: 392483
@[kprobe:prepare_to_wait_event]: 778690

As in calls to prepare_to_wait_event() almost double calls to
finish_wait(). This evens out with the patch.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---

One may worry about using "condition" twice. However, macros leading up
to this one already do it, so it should be fine.

Also one may wonder about fences -- to my understanding going off and on
CPU guarantees a full fence, so the now avoided lock trip changes
nothing.

 include/linux/wait.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/wait.h b/include/linux/wait.h
index 2bdc8f47963b..965a19809c7e 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -316,6 +316,9 @@ extern void init_wait_entry(struct wait_queue_entry *wq_entry, int flags);
 		}								\
 										\
 		cmd;								\
+										\
+		if (condition)							\
+			break;							\
 	}									\
 	finish_wait(&wq_head, &__wq_entry);					\
 __out:	__ret;									\
-- 
2.43.0


  parent reply	other threads:[~2025-03-03 23:04 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-03 23:04 [PATCH 0/3] some pipe + wait stuff Mateusz Guzik
2025-03-03 23:04 ` [PATCH 1/3] pipe: drop an always true check in anon_pipe_write() Mateusz Guzik
2025-03-04 14:07   ` Oleg Nesterov
2025-03-04 14:58     ` Mateusz Guzik
2025-03-04 15:18       ` Oleg Nesterov
2025-03-04 15:44     ` pipes && EPOLLET, again Oleg Nesterov
2025-03-04 18:12       ` Linus Torvalds
2025-03-04 19:32         ` Oleg Nesterov
2025-03-04 19:49           ` Linus Torvalds
2025-03-03 23:04 ` [PATCH 2/3] pipe: cache 2 pages instead of 1 Mateusz Guzik
2025-03-03 23:04 ` Mateusz Guzik [this message]
2025-03-04 14:19   ` [PATCH 3/3] wait: avoid spurious calls to prepare_to_wait_event() in ___wait_event() Peter Zijlstra
2025-03-04 15:25     ` Mateusz Guzik
2025-03-04  8:46 ` [PATCH 0/3] some pipe + wait stuff Christian Brauner

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=20250303230409.452687-4-mjguzik@gmail.com \
    --to=mjguzik@gmail.com \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    /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