arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++ include/linux/cpuhotplug.h | 1 + 2 files changed, 21 insertions(+)
When the cpu is offline, the timer under loongarch is not correctly closed,
resulting in an excessively high cpu usage rate of the offline vcpu thread
in the virtual machine.
To correctly close the timer, we have made the following modifications:
Register the cpu hotplug timer start event for loongarch.This event will
be called to close the timer when the cpu is offline.
Clear the timer interrupt when the timer is turned off
Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
---
arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++
include/linux/cpuhotplug.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c
index 367906b10f81..4daa11512eba 100644
--- a/arch/loongarch/kernel/time.c
+++ b/arch/loongarch/kernel/time.c
@@ -12,6 +12,7 @@
#include <linux/kernel.h>
#include <linux/sched_clock.h>
#include <linux/spinlock.h>
+#include <linux/cpu.h>
#include <asm/cpu-features.h>
#include <asm/loongarch.h>
@@ -86,6 +87,9 @@ static int constant_set_state_shutdown(struct clock_event_device *evt)
timer_config &= ~CSR_TCFG_EN;
csr_write64(timer_config, LOONGARCH_CSR_TCFG);
+ /* Clear Timer Interrupt */
+ write_csr_tintclear(CSR_TINTCLR_TI);
+
raw_spin_unlock(&state_lock);
return 0;
@@ -208,8 +212,17 @@ int __init constant_clocksource_init(void)
return res;
}
+static int arch_timer_dying_cpu(unsigned int cpu)
+{
+ constant_set_state_shutdown(NULL);
+
+ return 0;
+}
+
void __init time_init(void)
{
+ int err;
+
if (!cpu_has_cpucfg)
const_clock_freq = cpu_clock_freq;
else
@@ -220,4 +233,11 @@ void __init time_init(void)
constant_clockevent_init();
constant_clocksource_init();
pv_time_init();
+
+ err = cpuhp_setup_state_nocalls(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING,
+ "loongarch/timer:starting",
+ NULL, arch_timer_dying_cpu);
+ if (err)
+ pr_info("cpu hotplug event register failed");
+
}
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index edfa61d80702..6606c1546afc 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -159,6 +159,7 @@ enum cpuhp_state {
CPUHP_AP_PERF_ARM_STARTING,
CPUHP_AP_PERF_RISCV_STARTING,
CPUHP_AP_ARM_L2X0_STARTING,
+ CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING,
CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
CPUHP_AP_ARM_ARCH_TIMER_STARTING,
CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING,
base-commit: 53e760d8949895390e256e723e7ee46618310361
--
2.39.1
Hi, Xianglai, There is something that can be improved. On Tue, Aug 12, 2025 at 5:24 PM Xianglai Li <lixianglai@loongson.cn> wrote: > > When the cpu is offline, the timer under loongarch is not correctly closed, > resulting in an excessively high cpu usage rate of the offline vcpu thread > in the virtual machine. > > To correctly close the timer, we have made the following modifications: > > Register the cpu hotplug timer start event for loongarch.This event will > be called to close the timer when the cpu is offline. > > Clear the timer interrupt when the timer is turned off > > Signed-off-by: Xianglai Li <lixianglai@loongson.cn> > --- > arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++ > include/linux/cpuhotplug.h | 1 + > 2 files changed, 21 insertions(+) > > diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c > index 367906b10f81..4daa11512eba 100644 > --- a/arch/loongarch/kernel/time.c > +++ b/arch/loongarch/kernel/time.c > @@ -12,6 +12,7 @@ > #include <linux/kernel.h> > #include <linux/sched_clock.h> > #include <linux/spinlock.h> > +#include <linux/cpu.h> > > #include <asm/cpu-features.h> > #include <asm/loongarch.h> > @@ -86,6 +87,9 @@ static int constant_set_state_shutdown(struct clock_event_device *evt) > timer_config &= ~CSR_TCFG_EN; > csr_write64(timer_config, LOONGARCH_CSR_TCFG); > > + /* Clear Timer Interrupt */ > + write_csr_tintclear(CSR_TINTCLR_TI); > + > raw_spin_unlock(&state_lock); > > return 0; > @@ -208,8 +212,17 @@ int __init constant_clocksource_init(void) > return res; > } > > +static int arch_timer_dying_cpu(unsigned int cpu) We can use arch_timer_dying() for short. And then add an arch_timer_starting() like this: static int arch_timer_starting(unsigned int cpu) { set_csr_ecfg(ECFGF_TIMER); return 0; } Though ECFGF_TIMER may be enabled in other places, for syntax we need it here. > +{ > + constant_set_state_shutdown(NULL); > + > + return 0; > +} > + > void __init time_init(void) > { > + int err; > + > if (!cpu_has_cpucfg) > const_clock_freq = cpu_clock_freq; > else > @@ -220,4 +233,11 @@ void __init time_init(void) > constant_clockevent_init(); > constant_clocksource_init(); > pv_time_init(); > + > + err = cpuhp_setup_state_nocalls(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > + "loongarch/timer:starting", > + NULL, arch_timer_dying_cpu); Then we need use cpuhp_setup_state() here, because we have a startup function now. And "loongarch/timer:starting" should be "clockevents/loongarch/timer:starting" like others. And the whole should be moved to the last of constant_clockevent_init() because it is clockevent specific. > + if (err) > + pr_info("cpu hotplug event register failed"); This is not so useful, because the error isn't fatal. Huacai > + > } > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h > index edfa61d80702..6606c1546afc 100644 > --- a/include/linux/cpuhotplug.h > +++ b/include/linux/cpuhotplug.h > @@ -159,6 +159,7 @@ enum cpuhp_state { > CPUHP_AP_PERF_ARM_STARTING, > CPUHP_AP_PERF_RISCV_STARTING, > CPUHP_AP_ARM_L2X0_STARTING, > + CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, > CPUHP_AP_ARM_ARCH_TIMER_STARTING, > CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING, > > base-commit: 53e760d8949895390e256e723e7ee46618310361 > -- > 2.39.1 >
Hi Huacai Chen: > Hi, Xianglai, > > There is something that can be improved. > > On Tue, Aug 12, 2025 at 5:24 PM Xianglai Li <lixianglai@loongson.cn> wrote: >> When the cpu is offline, the timer under loongarch is not correctly closed, >> resulting in an excessively high cpu usage rate of the offline vcpu thread >> in the virtual machine. >> >> To correctly close the timer, we have made the following modifications: >> >> Register the cpu hotplug timer start event for loongarch.This event will >> be called to close the timer when the cpu is offline. >> >> Clear the timer interrupt when the timer is turned off >> >> Signed-off-by: Xianglai Li <lixianglai@loongson.cn> >> --- >> arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++ >> include/linux/cpuhotplug.h | 1 + >> 2 files changed, 21 insertions(+) >> >> diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c >> index 367906b10f81..4daa11512eba 100644 >> --- a/arch/loongarch/kernel/time.c >> +++ b/arch/loongarch/kernel/time.c >> @@ -12,6 +12,7 @@ >> #include <linux/kernel.h> >> #include <linux/sched_clock.h> >> #include <linux/spinlock.h> >> +#include <linux/cpu.h> >> >> #include <asm/cpu-features.h> >> #include <asm/loongarch.h> >> @@ -86,6 +87,9 @@ static int constant_set_state_shutdown(struct clock_event_device *evt) >> timer_config &= ~CSR_TCFG_EN; >> csr_write64(timer_config, LOONGARCH_CSR_TCFG); >> >> + /* Clear Timer Interrupt */ >> + write_csr_tintclear(CSR_TINTCLR_TI); >> + >> raw_spin_unlock(&state_lock); >> >> return 0; >> @@ -208,8 +212,17 @@ int __init constant_clocksource_init(void) >> return res; >> } >> >> +static int arch_timer_dying_cpu(unsigned int cpu) > We can use arch_timer_dying() for short. And then add an > arch_timer_starting() like this: > > static int arch_timer_starting(unsigned int cpu) > { > set_csr_ecfg(ECFGF_TIMER); > > return 0; > } > > Though ECFGF_TIMER may be enabled in other places, for syntax we need it here. > >> +{ >> + constant_set_state_shutdown(NULL); >> + >> + return 0; >> +} >> + >> void __init time_init(void) >> { >> + int err; >> + >> if (!cpu_has_cpucfg) >> const_clock_freq = cpu_clock_freq; >> else >> @@ -220,4 +233,11 @@ void __init time_init(void) >> constant_clockevent_init(); >> constant_clocksource_init(); >> pv_time_init(); >> + >> + err = cpuhp_setup_state_nocalls(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, >> + "loongarch/timer:starting", >> + NULL, arch_timer_dying_cpu); > Then we need use cpuhp_setup_state() here, because we have a startup > function now. > > And "loongarch/timer:starting" should be > "clockevents/loongarch/timer:starting" like others. > > And the whole should be moved to the last of > constant_clockevent_init() because it is clockevent specific. like this? @@ -164,6 +182,10 @@ int constant_clockevent_init(void) timer_irq_installed = 1; + cpuhp_setup_state(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, + "clockevents/loongarch/timer:starting", + arch_timer_starting, arch_timer_dying_cpu); + sync_counter(); I was wondering whether it should be placed before or after the "timer_irq_installed" judgment >> + if (err) >> + pr_info("cpu hotplug event register failed"); > This is not so useful, because the error isn't fatal. > > > Huacai > >> + >> } >> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h >> index edfa61d80702..6606c1546afc 100644 >> --- a/include/linux/cpuhotplug.h >> +++ b/include/linux/cpuhotplug.h >> @@ -159,6 +159,7 @@ enum cpuhp_state { >> CPUHP_AP_PERF_ARM_STARTING, >> CPUHP_AP_PERF_RISCV_STARTING, >> CPUHP_AP_ARM_L2X0_STARTING, >> + CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, >> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, >> CPUHP_AP_ARM_ARCH_TIMER_STARTING, >> CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING, >> >> base-commit: 53e760d8949895390e256e723e7ee46618310361 >> -- >> 2.39.1 >>
On Tue, Aug 12, 2025 at 8:50 PM lixianglai <lixianglai@loongson.cn> wrote: > > > > Hi Huacai Chen: > > Hi, Xianglai, > > > > There is something that can be improved. > > > > On Tue, Aug 12, 2025 at 5:24 PM Xianglai Li <lixianglai@loongson.cn> wrote: > >> When the cpu is offline, the timer under loongarch is not correctly closed, > >> resulting in an excessively high cpu usage rate of the offline vcpu thread > >> in the virtual machine. > >> > >> To correctly close the timer, we have made the following modifications: > >> > >> Register the cpu hotplug timer start event for loongarch.This event will > >> be called to close the timer when the cpu is offline. > >> > >> Clear the timer interrupt when the timer is turned off > >> > >> Signed-off-by: Xianglai Li <lixianglai@loongson.cn> > >> --- > >> arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++ > >> include/linux/cpuhotplug.h | 1 + > >> 2 files changed, 21 insertions(+) > >> > >> diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c > >> index 367906b10f81..4daa11512eba 100644 > >> --- a/arch/loongarch/kernel/time.c > >> +++ b/arch/loongarch/kernel/time.c > >> @@ -12,6 +12,7 @@ > >> #include <linux/kernel.h> > >> #include <linux/sched_clock.h> > >> #include <linux/spinlock.h> > >> +#include <linux/cpu.h> > >> > >> #include <asm/cpu-features.h> > >> #include <asm/loongarch.h> > >> @@ -86,6 +87,9 @@ static int constant_set_state_shutdown(struct clock_event_device *evt) > >> timer_config &= ~CSR_TCFG_EN; > >> csr_write64(timer_config, LOONGARCH_CSR_TCFG); > >> > >> + /* Clear Timer Interrupt */ > >> + write_csr_tintclear(CSR_TINTCLR_TI); > >> + > >> raw_spin_unlock(&state_lock); > >> > >> return 0; > >> @@ -208,8 +212,17 @@ int __init constant_clocksource_init(void) > >> return res; > >> } > >> > >> +static int arch_timer_dying_cpu(unsigned int cpu) > > We can use arch_timer_dying() for short. And then add an > > arch_timer_starting() like this: > > > > static int arch_timer_starting(unsigned int cpu) > > { > > set_csr_ecfg(ECFGF_TIMER); > > > > return 0; > > } > > > > Though ECFGF_TIMER may be enabled in other places, for syntax we need it here. > > > >> +{ > >> + constant_set_state_shutdown(NULL); > >> + > >> + return 0; > >> +} > >> + > >> void __init time_init(void) > >> { > >> + int err; > >> + > >> if (!cpu_has_cpucfg) > >> const_clock_freq = cpu_clock_freq; > >> else > >> @@ -220,4 +233,11 @@ void __init time_init(void) > >> constant_clockevent_init(); > >> constant_clocksource_init(); > >> pv_time_init(); > >> + > >> + err = cpuhp_setup_state_nocalls(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > >> + "loongarch/timer:starting", > >> + NULL, arch_timer_dying_cpu); > > Then we need use cpuhp_setup_state() here, because we have a startup > > function now. > > > > And "loongarch/timer:starting" should be > > "clockevents/loongarch/timer:starting" like others. > > > > And the whole should be moved to the last of > > constant_clockevent_init() because it is clockevent specific. > > like this? > > @@ -164,6 +182,10 @@ int constant_clockevent_init(void) > > timer_irq_installed = 1; > > + cpuhp_setup_state(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > + "clockevents/loongarch/timer:starting", > + arch_timer_starting, arch_timer_dying_cpu); > + > sync_counter(); > > > I was wondering whether it should be placed before or after the > "timer_irq_installed" judgment Should be after "timer_irq_installed" because we only need to run once. The best place is after pr_info("Constant clock event device register\n"); And there is another question: Should we move "write_csr_tintclear(CSR_TINTCLR_TI)" from constant_set_state_shutdown() to arch_timer_dying()? Huacai > > > >> + if (err) > >> + pr_info("cpu hotplug event register failed"); > > This is not so useful, because the error isn't fatal. > > > > > > Huacai > > > >> + > >> } > >> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h > >> index edfa61d80702..6606c1546afc 100644 > >> --- a/include/linux/cpuhotplug.h > >> +++ b/include/linux/cpuhotplug.h > >> @@ -159,6 +159,7 @@ enum cpuhp_state { > >> CPUHP_AP_PERF_ARM_STARTING, > >> CPUHP_AP_PERF_RISCV_STARTING, > >> CPUHP_AP_ARM_L2X0_STARTING, > >> + CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > >> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, > >> CPUHP_AP_ARM_ARCH_TIMER_STARTING, > >> CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING, > >> > >> base-commit: 53e760d8949895390e256e723e7ee46618310361 > >> -- > >> 2.39.1 > >> > >
Hi Huacai Chen: > On Tue, Aug 12, 2025 at 8:50 PM lixianglai <lixianglai@loongson.cn> wrote: >> >> >> Hi Huacai Chen: >>> Hi, Xianglai, >>> >>> There is something that can be improved. >>> >>> On Tue, Aug 12, 2025 at 5:24 PM Xianglai Li <lixianglai@loongson.cn> wrote: >>>> When the cpu is offline, the timer under loongarch is not correctly closed, >>>> resulting in an excessively high cpu usage rate of the offline vcpu thread >>>> in the virtual machine. >>>> >>>> To correctly close the timer, we have made the following modifications: >>>> >>>> Register the cpu hotplug timer start event for loongarch.This event will >>>> be called to close the timer when the cpu is offline. >>>> >>>> Clear the timer interrupt when the timer is turned off >>>> >>>> Signed-off-by: Xianglai Li <lixianglai@loongson.cn> >>>> --- >>>> arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++ >>>> include/linux/cpuhotplug.h | 1 + >>>> 2 files changed, 21 insertions(+) >>>> >>>> diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c >>>> index 367906b10f81..4daa11512eba 100644 >>>> --- a/arch/loongarch/kernel/time.c >>>> +++ b/arch/loongarch/kernel/time.c >>>> @@ -12,6 +12,7 @@ >>>> #include <linux/kernel.h> >>>> #include <linux/sched_clock.h> >>>> #include <linux/spinlock.h> >>>> +#include <linux/cpu.h> >>>> >>>> #include <asm/cpu-features.h> >>>> #include <asm/loongarch.h> >>>> @@ -86,6 +87,9 @@ static int constant_set_state_shutdown(struct clock_event_device *evt) >>>> timer_config &= ~CSR_TCFG_EN; >>>> csr_write64(timer_config, LOONGARCH_CSR_TCFG); >>>> >>>> + /* Clear Timer Interrupt */ >>>> + write_csr_tintclear(CSR_TINTCLR_TI); >>>> + >>>> raw_spin_unlock(&state_lock); >>>> >>>> return 0; >>>> @@ -208,8 +212,17 @@ int __init constant_clocksource_init(void) >>>> return res; >>>> } >>>> >>>> +static int arch_timer_dying_cpu(unsigned int cpu) >>> We can use arch_timer_dying() for short. And then add an >>> arch_timer_starting() like this: >>> >>> static int arch_timer_starting(unsigned int cpu) >>> { >>> set_csr_ecfg(ECFGF_TIMER); >>> >>> return 0; >>> } >>> >>> Though ECFGF_TIMER may be enabled in other places, for syntax we need it here. >>> >>>> +{ >>>> + constant_set_state_shutdown(NULL); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> void __init time_init(void) >>>> { >>>> + int err; >>>> + >>>> if (!cpu_has_cpucfg) >>>> const_clock_freq = cpu_clock_freq; >>>> else >>>> @@ -220,4 +233,11 @@ void __init time_init(void) >>>> constant_clockevent_init(); >>>> constant_clocksource_init(); >>>> pv_time_init(); >>>> + >>>> + err = cpuhp_setup_state_nocalls(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, >>>> + "loongarch/timer:starting", >>>> + NULL, arch_timer_dying_cpu); >>> Then we need use cpuhp_setup_state() here, because we have a startup >>> function now. >>> >>> And "loongarch/timer:starting" should be >>> "clockevents/loongarch/timer:starting" like others. >>> >>> And the whole should be moved to the last of >>> constant_clockevent_init() because it is clockevent specific. >> like this? >> >> @@ -164,6 +182,10 @@ int constant_clockevent_init(void) >> >> timer_irq_installed = 1; >> >> + cpuhp_setup_state(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, >> + "clockevents/loongarch/timer:starting", >> + arch_timer_starting, arch_timer_dying_cpu); >> + >> sync_counter(); >> >> >> I was wondering whether it should be placed before or after the >> "timer_irq_installed" judgment > Should be after "timer_irq_installed" because we only need to run once. > > The best place is after pr_info("Constant clock event device register\n"); Got it! > And there is another question: > Should we move "write_csr_tintclear(CSR_TINTCLR_TI)" from > constant_set_state_shutdown() to arch_timer_dying()? We may not need to do this. If the context in which the timer is turned off is to disable interrupts, and the last timing period just arrives during this process, when the interrupt is re-enabled, an additional interrupt will be triggered and the next timing expiration time will be filled. This is not in line with expectations. Isn't it more reasonable to clean up the interrupt flag after turning off the timer instead of just doing so cpu offline context cleaning. Thanks! Xianglai. > > Huacai > >> >>>> + if (err) >>>> + pr_info("cpu hotplug event register failed"); >>> This is not so useful, because the error isn't fatal. >>> >>> >>> Huacai >>> >>>> + >>>> } >>>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h >>>> index edfa61d80702..6606c1546afc 100644 >>>> --- a/include/linux/cpuhotplug.h >>>> +++ b/include/linux/cpuhotplug.h >>>> @@ -159,6 +159,7 @@ enum cpuhp_state { >>>> CPUHP_AP_PERF_ARM_STARTING, >>>> CPUHP_AP_PERF_RISCV_STARTING, >>>> CPUHP_AP_ARM_L2X0_STARTING, >>>> + CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, >>>> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, >>>> CPUHP_AP_ARM_ARCH_TIMER_STARTING, >>>> CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING, >>>> >>>> base-commit: 53e760d8949895390e256e723e7ee46618310361 >>>> -- >>>> 2.39.1 >>>> >>
Hi Huacai Chen: >> On Tue, Aug 12, 2025 at 8:50 PM lixianglai <lixianglai@loongson.cn> >> wrote: >>> >>> >>> Hi Huacai Chen: >>>> Hi, Xianglai, >>>> >>>> There is something that can be improved. >>>> >>>> On Tue, Aug 12, 2025 at 5:24 PM Xianglai Li >>>> <lixianglai@loongson.cn> wrote: >>>>> When the cpu is offline, the timer under loongarch is not >>>>> correctly closed, >>>>> resulting in an excessively high cpu usage rate of the offline >>>>> vcpu thread >>>>> in the virtual machine. >>>>> >>>>> To correctly close the timer, we have made the following >>>>> modifications: >>>>> >>>>> Register the cpu hotplug timer start event for loongarch.This >>>>> event will >>>>> be called to close the timer when the cpu is offline. >>>>> >>>>> Clear the timer interrupt when the timer is turned off >>>>> >>>>> Signed-off-by: Xianglai Li <lixianglai@loongson.cn> >>>>> --- >>>>> arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++ >>>>> include/linux/cpuhotplug.h | 1 + >>>>> 2 files changed, 21 insertions(+) >>>>> >>>>> diff --git a/arch/loongarch/kernel/time.c >>>>> b/arch/loongarch/kernel/time.c >>>>> index 367906b10f81..4daa11512eba 100644 >>>>> --- a/arch/loongarch/kernel/time.c >>>>> +++ b/arch/loongarch/kernel/time.c >>>>> @@ -12,6 +12,7 @@ >>>>> #include <linux/kernel.h> >>>>> #include <linux/sched_clock.h> >>>>> #include <linux/spinlock.h> >>>>> +#include <linux/cpu.h> >>>>> >>>>> #include <asm/cpu-features.h> >>>>> #include <asm/loongarch.h> >>>>> @@ -86,6 +87,9 @@ static int constant_set_state_shutdown(struct >>>>> clock_event_device *evt) >>>>> timer_config &= ~CSR_TCFG_EN; >>>>> csr_write64(timer_config, LOONGARCH_CSR_TCFG); >>>>> >>>>> + /* Clear Timer Interrupt */ >>>>> + write_csr_tintclear(CSR_TINTCLR_TI); >>>>> + >>>>> raw_spin_unlock(&state_lock); >>>>> >>>>> return 0; >>>>> @@ -208,8 +212,17 @@ int __init constant_clocksource_init(void) >>>>> return res; >>>>> } >>>>> >>>>> +static int arch_timer_dying_cpu(unsigned int cpu) >>>> We can use arch_timer_dying() for short. And then add an >>>> arch_timer_starting() like this: >>>> >>>> static int arch_timer_starting(unsigned int cpu) >>>> { >>>> set_csr_ecfg(ECFGF_TIMER); >>>> >>>> return 0; >>>> } >>>> >>>> Though ECFGF_TIMER may be enabled in other places, for syntax we >>>> need it here. >>>> >>>>> +{ >>>>> + constant_set_state_shutdown(NULL); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> void __init time_init(void) >>>>> { >>>>> + int err; >>>>> + >>>>> if (!cpu_has_cpucfg) >>>>> const_clock_freq = cpu_clock_freq; >>>>> else >>>>> @@ -220,4 +233,11 @@ void __init time_init(void) >>>>> constant_clockevent_init(); >>>>> constant_clocksource_init(); >>>>> pv_time_init(); >>>>> + >>>>> + err = >>>>> cpuhp_setup_state_nocalls(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, >>>>> + "loongarch/timer:starting", >>>>> + NULL, arch_timer_dying_cpu); >>>> Then we need use cpuhp_setup_state() here, because we have a startup >>>> function now. >>>> >>>> And "loongarch/timer:starting" should be >>>> "clockevents/loongarch/timer:starting" like others. >>>> >>>> And the whole should be moved to the last of >>>> constant_clockevent_init() because it is clockevent specific. >>> like this? >>> >>> @@ -164,6 +182,10 @@ int constant_clockevent_init(void) >>> >>> timer_irq_installed = 1; >>> >>> + cpuhp_setup_state(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, >>> + "clockevents/loongarch/timer:starting", >>> + arch_timer_starting, arch_timer_dying_cpu); >>> + >>> sync_counter(); >>> >>> >>> I was wondering whether it should be placed before or after the >>> "timer_irq_installed" judgment >> Should be after "timer_irq_installed" because we only need to run once. >> >> The best place is after pr_info("Constant clock event device >> register\n"); > > Got it! >> And there is another question: >> Should we move "write_csr_tintclear(CSR_TINTCLR_TI)" from >> constant_set_state_shutdown() to arch_timer_dying()? > > We may not need to do this. If the context in which the timer is > turned off is to disable interrupts, and the last timing period just > arrives during this process, when the interrupt is re-enabled, > an additional interrupt will be triggered and the next timing expiration > time will be filled. This is not in line with expectations. > Isn't it more reasonable to clean up the interrupt flag after turning off > the timer instead of just doing so cpu offline context cleaning. > This is my personal opinion, but I just trace the process of cpu offline and need to clear the timer interrupt. Because is not particularly proficient in of the overall system architecture, I might have a one-sided view. Do you have a better suggestion? Thanks! Xianglai. > Thanks! > Xianglai. > >> >> Huacai >> >>> >>>>> + if (err) >>>>> + pr_info("cpu hotplug event register failed"); >>>> This is not so useful, because the error isn't fatal. >>>> >>>> >>>> Huacai >>>> >>>>> + >>>>> } >>>>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h >>>>> index edfa61d80702..6606c1546afc 100644 >>>>> --- a/include/linux/cpuhotplug.h >>>>> +++ b/include/linux/cpuhotplug.h >>>>> @@ -159,6 +159,7 @@ enum cpuhp_state { >>>>> CPUHP_AP_PERF_ARM_STARTING, >>>>> CPUHP_AP_PERF_RISCV_STARTING, >>>>> CPUHP_AP_ARM_L2X0_STARTING, >>>>> + CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, >>>>> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, >>>>> CPUHP_AP_ARM_ARCH_TIMER_STARTING, >>>>> CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING, >>>>> >>>>> base-commit: 53e760d8949895390e256e723e7ee46618310361 >>>>> -- >>>>> 2.39.1 >>>>> >>> >
On Wed, Aug 13, 2025 at 10:27 AM lixianglai <lixianglai@loongson.cn> wrote: > > Hi Huacai Chen: > >> On Tue, Aug 12, 2025 at 8:50 PM lixianglai <lixianglai@loongson.cn> > >> wrote: > >>> > >>> > >>> Hi Huacai Chen: > >>>> Hi, Xianglai, > >>>> > >>>> There is something that can be improved. > >>>> > >>>> On Tue, Aug 12, 2025 at 5:24 PM Xianglai Li > >>>> <lixianglai@loongson.cn> wrote: > >>>>> When the cpu is offline, the timer under loongarch is not > >>>>> correctly closed, > >>>>> resulting in an excessively high cpu usage rate of the offline > >>>>> vcpu thread > >>>>> in the virtual machine. > >>>>> > >>>>> To correctly close the timer, we have made the following > >>>>> modifications: > >>>>> > >>>>> Register the cpu hotplug timer start event for loongarch.This > >>>>> event will > >>>>> be called to close the timer when the cpu is offline. > >>>>> > >>>>> Clear the timer interrupt when the timer is turned off > >>>>> > >>>>> Signed-off-by: Xianglai Li <lixianglai@loongson.cn> > >>>>> --- > >>>>> arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++ > >>>>> include/linux/cpuhotplug.h | 1 + > >>>>> 2 files changed, 21 insertions(+) > >>>>> > >>>>> diff --git a/arch/loongarch/kernel/time.c > >>>>> b/arch/loongarch/kernel/time.c > >>>>> index 367906b10f81..4daa11512eba 100644 > >>>>> --- a/arch/loongarch/kernel/time.c > >>>>> +++ b/arch/loongarch/kernel/time.c > >>>>> @@ -12,6 +12,7 @@ > >>>>> #include <linux/kernel.h> > >>>>> #include <linux/sched_clock.h> > >>>>> #include <linux/spinlock.h> > >>>>> +#include <linux/cpu.h> > >>>>> > >>>>> #include <asm/cpu-features.h> > >>>>> #include <asm/loongarch.h> > >>>>> @@ -86,6 +87,9 @@ static int constant_set_state_shutdown(struct > >>>>> clock_event_device *evt) > >>>>> timer_config &= ~CSR_TCFG_EN; > >>>>> csr_write64(timer_config, LOONGARCH_CSR_TCFG); > >>>>> > >>>>> + /* Clear Timer Interrupt */ > >>>>> + write_csr_tintclear(CSR_TINTCLR_TI); > >>>>> + > >>>>> raw_spin_unlock(&state_lock); > >>>>> > >>>>> return 0; > >>>>> @@ -208,8 +212,17 @@ int __init constant_clocksource_init(void) > >>>>> return res; > >>>>> } > >>>>> > >>>>> +static int arch_timer_dying_cpu(unsigned int cpu) > >>>> We can use arch_timer_dying() for short. And then add an > >>>> arch_timer_starting() like this: > >>>> > >>>> static int arch_timer_starting(unsigned int cpu) > >>>> { > >>>> set_csr_ecfg(ECFGF_TIMER); > >>>> > >>>> return 0; > >>>> } > >>>> > >>>> Though ECFGF_TIMER may be enabled in other places, for syntax we > >>>> need it here. > >>>> > >>>>> +{ > >>>>> + constant_set_state_shutdown(NULL); > >>>>> + > >>>>> + return 0; > >>>>> +} > >>>>> + > >>>>> void __init time_init(void) > >>>>> { > >>>>> + int err; > >>>>> + > >>>>> if (!cpu_has_cpucfg) > >>>>> const_clock_freq = cpu_clock_freq; > >>>>> else > >>>>> @@ -220,4 +233,11 @@ void __init time_init(void) > >>>>> constant_clockevent_init(); > >>>>> constant_clocksource_init(); > >>>>> pv_time_init(); > >>>>> + > >>>>> + err = > >>>>> cpuhp_setup_state_nocalls(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > >>>>> + "loongarch/timer:starting", > >>>>> + NULL, arch_timer_dying_cpu); > >>>> Then we need use cpuhp_setup_state() here, because we have a startup > >>>> function now. > >>>> > >>>> And "loongarch/timer:starting" should be > >>>> "clockevents/loongarch/timer:starting" like others. > >>>> > >>>> And the whole should be moved to the last of > >>>> constant_clockevent_init() because it is clockevent specific. > >>> like this? > >>> > >>> @@ -164,6 +182,10 @@ int constant_clockevent_init(void) > >>> > >>> timer_irq_installed = 1; > >>> > >>> + cpuhp_setup_state(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > >>> + "clockevents/loongarch/timer:starting", > >>> + arch_timer_starting, arch_timer_dying_cpu); > >>> + > >>> sync_counter(); > >>> > >>> > >>> I was wondering whether it should be placed before or after the > >>> "timer_irq_installed" judgment > >> Should be after "timer_irq_installed" because we only need to run once. > >> > >> The best place is after pr_info("Constant clock event device > >> register\n"); > > > > Got it! > >> And there is another question: > >> Should we move "write_csr_tintclear(CSR_TINTCLR_TI)" from > >> constant_set_state_shutdown() to arch_timer_dying()? > > > > We may not need to do this. If the context in which the timer is > > turned off is to disable interrupts, and the last timing period just > > arrives during this process, when the interrupt is re-enabled, > > an additional interrupt will be triggered and the next timing expiration > > time will be filled. This is not in line with expectations. > > Isn't it more reasonable to clean up the interrupt flag after turning off > > the timer instead of just doing so cpu offline context cleaning. > > > > This is my personal opinion, but I just trace the process of cpu offline > and need to clear the timer interrupt. > Because is not particularly proficient in of the overall system > architecture, I might have a one-sided view. > Do you have a better suggestion? Without this patch constant_set_state_shutdown() seems work well, so I think clearing interrupt may only needed by cpuhotplug. The reason is that the timer interrupt handler doesn't work when the cpu is offline, but not in other cases. However, whether the clearing is in constant_set_state_shutdown() or not are both OK for me. Huacai > > Thanks! > Xianglai. > > > Thanks! > > Xianglai. > > > >> > >> Huacai > >> > >>> > >>>>> + if (err) > >>>>> + pr_info("cpu hotplug event register failed"); > >>>> This is not so useful, because the error isn't fatal. > >>>> > >>>> > >>>> Huacai > >>>> > >>>>> + > >>>>> } > >>>>> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h > >>>>> index edfa61d80702..6606c1546afc 100644 > >>>>> --- a/include/linux/cpuhotplug.h > >>>>> +++ b/include/linux/cpuhotplug.h > >>>>> @@ -159,6 +159,7 @@ enum cpuhp_state { > >>>>> CPUHP_AP_PERF_ARM_STARTING, > >>>>> CPUHP_AP_PERF_RISCV_STARTING, > >>>>> CPUHP_AP_ARM_L2X0_STARTING, > >>>>> + CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > >>>>> CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, > >>>>> CPUHP_AP_ARM_ARCH_TIMER_STARTING, > >>>>> CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING, > >>>>> > >>>>> base-commit: 53e760d8949895390e256e723e7ee46618310361 > >>>>> -- > >>>>> 2.39.1 > >>>>> > >>> > > >
On 2025/8/12 下午5:00, Xianglai Li wrote: > When the cpu is offline, the timer under loongarch is not correctly closed, > resulting in an excessively high cpu usage rate of the offline vcpu thread > in the virtual machine. > > To correctly close the timer, we have made the following modifications: > > Register the cpu hotplug timer start event for loongarch.This event will > be called to close the timer when the cpu is offline. > > Clear the timer interrupt when the timer is turned off During timer is turned off, timer interrupt may be arriving. Here adding EOI with timer after disabling it so that there is no pending timer interrupt when offline cpu is halted state. > > Signed-off-by: Xianglai Li <lixianglai@loongson.cn> > --- > arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++ > include/linux/cpuhotplug.h | 1 + > 2 files changed, 21 insertions(+) > > diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c > index 367906b10f81..4daa11512eba 100644 > --- a/arch/loongarch/kernel/time.c > +++ b/arch/loongarch/kernel/time.c > @@ -12,6 +12,7 @@ > #include <linux/kernel.h> > #include <linux/sched_clock.h> > #include <linux/spinlock.h> > +#include <linux/cpu.h> > > #include <asm/cpu-features.h> > #include <asm/loongarch.h> > @@ -86,6 +87,9 @@ static int constant_set_state_shutdown(struct clock_event_device *evt) > timer_config &= ~CSR_TCFG_EN; > csr_write64(timer_config, LOONGARCH_CSR_TCFG); > > + /* Clear Timer Interrupt */ > + write_csr_tintclear(CSR_TINTCLR_TI); > + > raw_spin_unlock(&state_lock); > > return 0; > @@ -208,8 +212,17 @@ int __init constant_clocksource_init(void) > return res; > } > > +static int arch_timer_dying_cpu(unsigned int cpu) > +{ NULL is a little strange here, how about something like this? struct clock_event_device *evt; evt = this_cpu_ptr(constant_clockevent_device); constant_set_state_shutdown(evt); Regards Bibo Mao > + constant_set_state_shutdown(NULL); > + > + return 0; > +} > + > void __init time_init(void) > { > + int err; > + > if (!cpu_has_cpucfg) > const_clock_freq = cpu_clock_freq; > else > @@ -220,4 +233,11 @@ void __init time_init(void) > constant_clockevent_init(); > constant_clocksource_init(); > pv_time_init(); > + > + err = cpuhp_setup_state_nocalls(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > + "loongarch/timer:starting", > + NULL, arch_timer_dying_cpu); > + if (err) > + pr_info("cpu hotplug event register failed"); > + > } > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h > index edfa61d80702..6606c1546afc 100644 > --- a/include/linux/cpuhotplug.h > +++ b/include/linux/cpuhotplug.h > @@ -159,6 +159,7 @@ enum cpuhp_state { > CPUHP_AP_PERF_ARM_STARTING, > CPUHP_AP_PERF_RISCV_STARTING, > CPUHP_AP_ARM_L2X0_STARTING, > + CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, > CPUHP_AP_ARM_ARCH_TIMER_STARTING, > CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING, > > base-commit: 53e760d8949895390e256e723e7ee46618310361 >
On Tue, Aug 12, 2025 at 6:50 PM Bibo Mao <maobibo@loongson.cn> wrote: > > > > On 2025/8/12 下午5:00, Xianglai Li wrote: > > When the cpu is offline, the timer under loongarch is not correctly closed, > > resulting in an excessively high cpu usage rate of the offline vcpu thread > > in the virtual machine. > > > > To correctly close the timer, we have made the following modifications: > > > > Register the cpu hotplug timer start event for loongarch.This event will > > be called to close the timer when the cpu is offline. > > > > Clear the timer interrupt when the timer is turned off > During timer is turned off, timer interrupt may be arriving. Here adding > EOI with timer after disabling it so that there is no pending timer > interrupt when offline cpu is halted state. > > > > Signed-off-by: Xianglai Li <lixianglai@loongson.cn> > > --- > > arch/loongarch/kernel/time.c | 20 ++++++++++++++++++++ > > include/linux/cpuhotplug.h | 1 + > > 2 files changed, 21 insertions(+) > > > > diff --git a/arch/loongarch/kernel/time.c b/arch/loongarch/kernel/time.c > > index 367906b10f81..4daa11512eba 100644 > > --- a/arch/loongarch/kernel/time.c > > +++ b/arch/loongarch/kernel/time.c > > @@ -12,6 +12,7 @@ > > #include <linux/kernel.h> > > #include <linux/sched_clock.h> > > #include <linux/spinlock.h> > > +#include <linux/cpu.h> > > > > #include <asm/cpu-features.h> > > #include <asm/loongarch.h> > > @@ -86,6 +87,9 @@ static int constant_set_state_shutdown(struct clock_event_device *evt) > > timer_config &= ~CSR_TCFG_EN; > > csr_write64(timer_config, LOONGARCH_CSR_TCFG); > > > > + /* Clear Timer Interrupt */ > > + write_csr_tintclear(CSR_TINTCLR_TI); > > + > > raw_spin_unlock(&state_lock); > > > > return 0; > > @@ -208,8 +212,17 @@ int __init constant_clocksource_init(void) > > return res; > > } > > > > +static int arch_timer_dying_cpu(unsigned int cpu) > > +{ > NULL is a little strange here, how about something like this? > > struct clock_event_device *evt; > > evt = this_cpu_ptr(constant_clockevent_device); > constant_set_state_shutdown(evt); Both OK for me, if really needed, we can use constant_set_state_shutdown(this_cpu_ptr(constant_clockevent_device)) for short. Huacai > > Regards > Bibo Mao > > > + constant_set_state_shutdown(NULL); > > + > > + return 0; > > +} > > + > > void __init time_init(void) > > { > > + int err; > > + > > if (!cpu_has_cpucfg) > > const_clock_freq = cpu_clock_freq; > > else > > @@ -220,4 +233,11 @@ void __init time_init(void) > > constant_clockevent_init(); > > constant_clocksource_init(); > > pv_time_init(); > > + > > + err = cpuhp_setup_state_nocalls(CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > > + "loongarch/timer:starting", > > + NULL, arch_timer_dying_cpu); > > + if (err) > > + pr_info("cpu hotplug event register failed"); > > + > > } > > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h > > index edfa61d80702..6606c1546afc 100644 > > --- a/include/linux/cpuhotplug.h > > +++ b/include/linux/cpuhotplug.h > > @@ -159,6 +159,7 @@ enum cpuhp_state { > > CPUHP_AP_PERF_ARM_STARTING, > > CPUHP_AP_PERF_RISCV_STARTING, > > CPUHP_AP_ARM_L2X0_STARTING, > > + CPUHP_AP_LOONGARCH_ARCH_TIMER_STARTING, > > CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, > > CPUHP_AP_ARM_ARCH_TIMER_STARTING, > > CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING, > > > > base-commit: 53e760d8949895390e256e723e7ee46618310361 > > > >
© 2016 - 2025 Red Hat, Inc.