2024-02-08 20:17:10

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH v2 2/5] sched/vtime: get rid of generic vtime_task_switch() implementation

The generic vtime_task_switch() implementation gets built only
if __ARCH_HAS_VTIME_TASK_SWITCH is not defined, but requires an
architecture to implement arch_vtime_task_switch() callback at
the same time, which is confusing.

Further, arch_vtime_task_switch() is implemented for 32-bit PowerPC
architecture only and vtime_task_switch() generic variant is rather
superfluous.

Simplify the whole vtime_task_switch() wiring by moving the existing
generic implementation to PowerPC.

Reviewed-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
arch/powerpc/include/asm/cputime.h | 13 -------------
arch/powerpc/kernel/time.c | 22 ++++++++++++++++++++++
kernel/sched/cputime.c | 13 -------------
3 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 4961fb38e438..aff858ca99c0 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -32,23 +32,10 @@
#ifdef CONFIG_PPC64
#define get_accounting(tsk) (&get_paca()->accounting)
#define raw_get_accounting(tsk) (&local_paca->accounting)
-static inline void arch_vtime_task_switch(struct task_struct *tsk) { }

#else
#define get_accounting(tsk) (&task_thread_info(tsk)->accounting)
#define raw_get_accounting(tsk) get_accounting(tsk)
-/*
- * Called from the context switch with interrupts disabled, to charge all
- * accumulated times to the current process, and to prepare accounting on
- * the next process.
- */
-static inline void arch_vtime_task_switch(struct task_struct *prev)
-{
- struct cpu_accounting_data *acct = get_accounting(current);
- struct cpu_accounting_data *acct0 = get_accounting(prev);
-
- acct->starttime = acct0->starttime;
-}
#endif

/*
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index df20cf201f74..c0fdc6d94fee 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -354,6 +354,28 @@ void vtime_flush(struct task_struct *tsk)
acct->hardirq_time = 0;
acct->softirq_time = 0;
}
+
+/*
+ * Called from the context switch with interrupts disabled, to charge all
+ * accumulated times to the current process, and to prepare accounting on
+ * the next process.
+ */
+void vtime_task_switch(struct task_struct *prev)
+{
+ if (is_idle_task(prev))
+ vtime_account_idle(prev);
+ else
+ vtime_account_kernel(prev);
+
+ vtime_flush(prev);
+
+ if (!IS_ENABLED(CONFIG_PPC64)) {
+ struct cpu_accounting_data *acct = get_accounting(current);
+ struct cpu_accounting_data *acct0 = get_accounting(prev);
+
+ acct->starttime = acct0->starttime;
+ }
+}
#endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */

void __no_kcsan __delay(unsigned long loops)
diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index af7952f12e6c..aa48b2ec879d 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -424,19 +424,6 @@ static inline void irqtime_account_process_tick(struct task_struct *p, int user_
*/
#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE

-# ifndef __ARCH_HAS_VTIME_TASK_SWITCH
-void vtime_task_switch(struct task_struct *prev)
-{
- if (is_idle_task(prev))
- vtime_account_idle(prev);
- else
- vtime_account_kernel(prev);
-
- vtime_flush(prev);
- arch_vtime_task_switch(prev);
-}
-# endif
-
void vtime_account_irq(struct task_struct *tsk, unsigned int offset)
{
unsigned int pc = irq_count() - offset;
--
2.40.1



2024-02-09 03:36:26

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] sched/vtime: get rid of generic vtime_task_switch() implementation

On Fri Feb 9, 2024 at 6:15 AM AEST, Alexander Gordeev wrote:
> The generic vtime_task_switch() implementation gets built only
> if __ARCH_HAS_VTIME_TASK_SWITCH is not defined, but requires an
> architecture to implement arch_vtime_task_switch() callback at
> the same time, which is confusing.
>
> Further, arch_vtime_task_switch() is implemented for 32-bit PowerPC
> architecture only and vtime_task_switch() generic variant is rather
> superfluous.
>
> Simplify the whole vtime_task_switch() wiring by moving the existing
> generic implementation to PowerPC.

I think this could be squashed with patch 3.

Reviewed-by: Nicholas Piggin <[email protected]>

>
> Reviewed-by: Frederic Weisbecker <[email protected]>
> Signed-off-by: Alexander Gordeev <[email protected]>
> ---
> arch/powerpc/include/asm/cputime.h | 13 -------------
> arch/powerpc/kernel/time.c | 22 ++++++++++++++++++++++
> kernel/sched/cputime.c | 13 -------------
> 3 files changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
> index 4961fb38e438..aff858ca99c0 100644
> --- a/arch/powerpc/include/asm/cputime.h
> +++ b/arch/powerpc/include/asm/cputime.h
> @@ -32,23 +32,10 @@
> #ifdef CONFIG_PPC64
> #define get_accounting(tsk) (&get_paca()->accounting)
> #define raw_get_accounting(tsk) (&local_paca->accounting)
> -static inline void arch_vtime_task_switch(struct task_struct *tsk) { }
>
> #else
> #define get_accounting(tsk) (&task_thread_info(tsk)->accounting)
> #define raw_get_accounting(tsk) get_accounting(tsk)
> -/*
> - * Called from the context switch with interrupts disabled, to charge all
> - * accumulated times to the current process, and to prepare accounting on
> - * the next process.
> - */
> -static inline void arch_vtime_task_switch(struct task_struct *prev)
> -{
> - struct cpu_accounting_data *acct = get_accounting(current);
> - struct cpu_accounting_data *acct0 = get_accounting(prev);
> -
> - acct->starttime = acct0->starttime;
> -}
> #endif
>
> /*
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index df20cf201f74..c0fdc6d94fee 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -354,6 +354,28 @@ void vtime_flush(struct task_struct *tsk)
> acct->hardirq_time = 0;
> acct->softirq_time = 0;
> }
> +
> +/*
> + * Called from the context switch with interrupts disabled, to charge all
> + * accumulated times to the current process, and to prepare accounting on
> + * the next process.
> + */
> +void vtime_task_switch(struct task_struct *prev)
> +{
> + if (is_idle_task(prev))
> + vtime_account_idle(prev);
> + else
> + vtime_account_kernel(prev);
> +
> + vtime_flush(prev);
> +
> + if (!IS_ENABLED(CONFIG_PPC64)) {
> + struct cpu_accounting_data *acct = get_accounting(current);
> + struct cpu_accounting_data *acct0 = get_accounting(prev);
> +
> + acct->starttime = acct0->starttime;
> + }
> +}
> #endif /* CONFIG_VIRT_CPU_ACCOUNTING_NATIVE */
>
> void __no_kcsan __delay(unsigned long loops)
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index af7952f12e6c..aa48b2ec879d 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -424,19 +424,6 @@ static inline void irqtime_account_process_tick(struct task_struct *p, int user_
> */
> #ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
>
> -# ifndef __ARCH_HAS_VTIME_TASK_SWITCH
> -void vtime_task_switch(struct task_struct *prev)
> -{
> - if (is_idle_task(prev))
> - vtime_account_idle(prev);
> - else
> - vtime_account_kernel(prev);
> -
> - vtime_flush(prev);
> - arch_vtime_task_switch(prev);
> -}
> -# endif
> -
> void vtime_account_irq(struct task_struct *tsk, unsigned int offset)
> {
> unsigned int pc = irq_count() - offset;