Re: [PATCH] hrtimer: Add a missing bracket and hide `migration_base' on !SMP
From: Thomas Gleixner
Date: Wed Sep 04 2019 - 10:39:24 EST
On Wed, 4 Sep 2019, Sebastian Andrzej Siewior wrote:
> 68b2c8c1e4210 ("hrtimer: Don't take expiry_lock when timer is currently migrated")
That is the same information as in the fixes tag. So you can say something
The recent change to avoid taking the expiry lock when a timer is currently
migrated missed .....
> missed to add a bracket at the end of the if statement leading to
> compile errors.
> Since that commit the variable `migration_base' is always used but only
> available on SMP configuration thus leading to another compile error.
> The changelog says
> "The timer base and base->cpu_base cannot be NULL in the code path"
> so it is safe to limit this check to SMP configurations only.
> Add the missing bracket to the if statement and hide `migration_base'
> behind CONFIG_SMP bars.
> Fixes: 68b2c8c1e4210 ("hrtimer: Don't take expiry_lock when timer is currently migrated")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> kernel/time/hrtimer.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
> diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
> index f5a1a5e16216c..bc84c74ae5b96 100644
> --- a/kernel/time/hrtimer.c
> +++ b/kernel/time/hrtimer.c
> @@ -1216,12 +1216,16 @@ void hrtimer_cancel_wait_running(const struct hrtimer *timer)
> /* Lockless read. Prevent the compiler from reloading it below */
> struct hrtimer_clock_base *base = READ_ONCE(timer->base);
> + bool is_migration_base = false;
> * Just relax if the timer expires in hard interrupt context or if
> * it is currently on the migration base.
> - if (!timer->is_soft || base == &migration_base)
> +#ifdef CONFIG_SMP
> + is_migration_base = base == &migration_base;
That's beyond ugly.
What's wrong with:
if (!timer->is_soft || is_migration_base(base))
and have two helpers in the relevant ifdeffed section?