Re: [PATCH 3/3] wait: avoid spurious calls to prepare_to_wait_event() in ___wait_event()

[Peter Zijlstra <peterz@infradead.org>]

On Tue, Mar 04, 2025 at 12:04:09AM +0100, Mateusz Guzik wrote:
> 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.

so it always helps if you provide actual numbers. Supposedly this makes
it go faster?

Also, how much bytes does it add to the image?

Anyway, no real objection, but it would be good to have better
substantiation etc.

>  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
>