Re: [patch V2 7/9] softirq: Replace barrier() with cpu_relax() in tasklet_unlock_wait()

[Peter Zijlstra <peterz@infradead.org>]

On Fri, Dec 04, 2020 at 06:01:58PM +0100, Thomas Gleixner wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> A barrier() in a tight loop which waits for something to happen on a remote
> CPU is a pointless exercise. Replace it with cpu_relax() which allows HT
> siblings to make progress.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  include/linux/interrupt.h |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -668,7 +668,8 @@ static inline void tasklet_unlock(struct
>  
>  static inline void tasklet_unlock_wait(struct tasklet_struct *t)
>  {
> -	while (test_bit(TASKLET_STATE_RUN, &(t)->state)) { barrier(); }
> +	while (test_bit(TASKLET_STATE_RUN, &(t)->state))
> +		cpu_relax();
>  }

Wouldn't it be nicer to stick a completion in tasklet_struct ? Or at the
very least use wait_var_event() or something?


diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index ee8299eb1f52..7818085ac003 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -663,12 +663,14 @@ static inline int tasklet_trylock(struct tasklet_struct *t)
 static inline void tasklet_unlock(struct tasklet_struct *t)
 {
 	smp_mb__before_atomic();
-	clear_bit(TASKLET_STATE_RUN, &(t)->state);
+	clear_bit(TASKLET_STATE_RUN, &t->state);
+	smp_mb__after_atomic();
+	wake_up_var(&t->state);
 }
 
 static inline void tasklet_unlock_wait(struct tasklet_struct *t)
 {
-	while (test_bit(TASKLET_STATE_RUN, &(t)->state)) { barrier(); }
+	wait_var_event(&t->state, !test_bit(TASKLET_STATE_RUN, &t->state));
 }
 #else
 #define tasklet_trylock(t) 1