Received-SPF: pass (google.com: domain of linux-kernel+bounces-37588-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: Thomas Gleixner <tglx@linutronix.de>
To: Yu Liao <liaoyu15@huawei.com>, linux-kernel@vger.kernel.org
Cc: liwei391@huawei.com, wangxiongfeng2@huawei.com, frederic@kernel.org,
 mingo@kernel.org
Subject: Re: [PATCH] tick/broadcast-hrtimer: Prevent the timer device on
 broadcast duty CPU from being disabled
In-Reply-To: <20231218025844.55675-1-liaoyu15@huawei.com>
References: <20231218025844.55675-1-liaoyu15@huawei.com>
Date: Wed, 24 Jan 2024 20:18:16 +0100
Message-ID: <87y1cek0av.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

On Mon, Dec 18 2023 at 10:58, Yu Liao wrote:
>  static int bc_set_next(ktime_t expires, struct clock_event_device *bc)
>  {
> +	ktime_t next_event = this_cpu_ptr(&tick_cpu_device)->evtdev->next_event;
> +
>  	/*
> -	 * This is called either from enter/exit idle code or from the
> -	 * broadcast handler. In all cases tick_broadcast_lock is held.
> -	 *
> +	 * This can be called from CPU offline operation to move broadcast
> +	 * assignment. If tick_broadcast_force_mask is set, the CPU local
> +	 * timer device may be disabled. And hrtimer_reprogram() will not
> +	 * called if the timer is not the first expiring timer. Reprogram
> +	 * the cpu local timer device to ensure we can take over the
> +	 * broadcast duty.
> +	 */
> +	if (tick_check_broadcast_expired() && expires >= next_event)
> +		tick_program_event(next_event, 1);

I'm not really enthused about another conditional here and that
condition is more than obscure.

The problem is that the local clockevent might be shut down, right?

So checking for that state is the right thing to do and the proper place
is in hotplug_cpu__broadcast_tick_pull(), no?

Thanks,

        tglx