linux-kernel - Re: [PATCH] tick: Remove unreasonable detached state set in tick

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e08270ec-1151-9119-ba06-bf56352d2029@loongson.cn>
Date: Mon, 11 Aug 2025 11:57:20 +0800
From: Bibo Mao <maobibo@...ngson.cn>
To: Anna-Maria Behnsen <anna-maria@...utronix.de>,
 Frederic Weisbecker <frederic@...nel.org>, Ingo Molnar <mingo@...nel.org>,
 Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tick: Remove unreasonable detached state set in
 tick_shutdown()

Hi,

Previously tick_shutdown() is called in BP, percpu timer cannot be 
shutdown in BP. Now it is called in AP, percpu timer can be shutdown in 
AP itself.

And there is another method, like other architectures add hotplug event 
callbak to shutdown percpu timer such as CPUHP_AP_LOONGARCH_TIMER_STARTING.

Which one do you prefer?

Regards
Bibo Mao

On 2025/8/7 上午11:59, Bibo Mao wrote:
> Function clockevents_switch_state() will check whether it has already
> switched to specified state, do nothing if it has.
> 
> In function tick_shutdown(), it will set detached state at first and
> call clockevents_switch_state() in clockevents_exchange_device(). The
> function clockevents_switch_state() will do nothing since it is already
> detached state. So the tick timer device will not be shutdown when CPU
> is offline. In guest VM system, timer interrupt will prevent vCPU to sleep
> if vCPU is hot removed.
> 
> Here remove state set before calling clockevents_exchange_device(),
> its state will be set in function clockevents_switch_state() if it
> succeeds to do so.
> 
> Signed-off-by: Bibo Mao <maobibo@...ngson.cn>
> ---
>   kernel/time/tick-common.c | 5 -----
>   1 file changed, 5 deletions(-)
> 
> diff --git a/kernel/time/tick-common.c b/kernel/time/tick-common.c
> index 9a3859443c04..eb9b777f5492 100644
> --- a/kernel/time/tick-common.c
> +++ b/kernel/time/tick-common.c
> @@ -424,11 +424,6 @@ void tick_shutdown(unsigned int cpu)
>   
>   	td->mode = TICKDEV_MODE_PERIODIC;
>   	if (dev) {
> -		/*
> -		 * Prevent that the clock events layer tries to call
> -		 * the set mode function!
> -		 */
> -		clockevent_set_state(dev, CLOCK_EVT_STATE_DETACHED);
>   		clockevents_exchange_device(dev, NULL);
>   		dev->event_handler = clockevents_handle_noop;
>   		td->evtdev = NULL;
> 
> base-commit: 7e161a991ea71e6ec526abc8f40c6852ebe3d946
>