From: Michael Ellerman <mpe@ellerman.id.au>
To: Alexander Gordeev <agordeev@linux.ibm.com>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>
Cc: linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org,
Nicholas Piggin <npiggin@gmail.com>,
Heiko Carstens <hca@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Frederic Weisbecker <frederic@kernel.org>
Subject: Re: [PATCH v2 RESEND 2/5] sched/vtime: get rid of generic vtime_task_switch() implementation
Date: Fri, 23 Feb 2024 08:46:03 +1100 [thread overview]
Message-ID: <87zfvs9nqc.fsf@mail.lhotse> (raw)
In-Reply-To: <5e8f7cfc5b405b892d55c51023e8149dfd83b253.1708612016.git.agordeev@linux.ibm.com>
Alexander Gordeev <agordeev@linux.ibm.com> writes:
> 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 <frederic@kernel.org>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
> ---
> 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(-)
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
> 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
WARNING: multiple messages have this Message-ID (diff)
From: Michael Ellerman <mpe@ellerman.id.au>
To: Alexander Gordeev <agordeev@linux.ibm.com>,
Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>
Cc: linux-s390@vger.kernel.org, Vasily Gorbik <gor@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
linux-kernel@vger.kernel.org, Nicholas Piggin <npiggin@gmail.com>,
linuxppc-dev@lists.ozlabs.org,
Frederic Weisbecker <frederic@kernel.org>
Subject: Re: [PATCH v2 RESEND 2/5] sched/vtime: get rid of generic vtime_task_switch() implementation
Date: Fri, 23 Feb 2024 08:46:03 +1100 [thread overview]
Message-ID: <87zfvs9nqc.fsf@mail.lhotse> (raw)
In-Reply-To: <5e8f7cfc5b405b892d55c51023e8149dfd83b253.1708612016.git.agordeev@linux.ibm.com>
Alexander Gordeev <agordeev@linux.ibm.com> writes:
> 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 <frederic@kernel.org>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Alexander Gordeev <agordeev@linux.ibm.com>
> ---
> 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(-)
Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
cheers
> 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
next prev parent reply other threads:[~2024-02-22 21:46 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-22 14:33 [PATCH v2 RESEND 0/5] sched/vtime: vtime.h headers cleanup Alexander Gordeev
2024-02-22 14:33 ` Alexander Gordeev
2024-02-22 14:33 ` [PATCH v2 RESEND 1/5] sched/vtime: remove confusing arch_vtime_task_switch() declaration Alexander Gordeev
2024-02-22 14:33 ` Alexander Gordeev
2024-02-22 14:33 ` [PATCH v2 RESEND 2/5] sched/vtime: get rid of generic vtime_task_switch() implementation Alexander Gordeev
2024-02-22 14:33 ` Alexander Gordeev
2024-02-22 21:46 ` Michael Ellerman [this message]
2024-02-22 21:46 ` Michael Ellerman
2024-02-22 14:33 ` [PATCH v2 RESEND 3/5] s390/vtime: remove unused __ARCH_HAS_VTIME_TASK_SWITCH leftover Alexander Gordeev
2024-02-22 14:33 ` Alexander Gordeev
2024-02-22 14:33 ` [PATCH v2 RESEND 4/5] s390/irq,nmi: include <asm/vtime.h> header directly Alexander Gordeev
2024-02-22 14:33 ` Alexander Gordeev
2024-02-22 14:33 ` [PATCH v2 RESEND 5/5] sched/vtime: do not include <asm/vtime.h> header Alexander Gordeev
2024-02-22 14:33 ` Alexander Gordeev
-- strict thread matches above, loose matches on Subject: below --
2024-04-10 15:09 [PATCH v2 RESEND 0/5] sched/vtime: vtime.h headers cleanup Alexander Gordeev
2024-04-10 15:09 ` [PATCH v2 RESEND 2/5] sched/vtime: get rid of generic vtime_task_switch() implementation Alexander Gordeev
2024-04-10 15:09 ` Alexander Gordeev
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zfvs9nqc.fsf@mail.lhotse \
--to=mpe@ellerman.id.au \
--cc=agordeev@linux.ibm.com \
--cc=frederic@kernel.org \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@redhat.com \
--cc=npiggin@gmail.com \
--cc=peterz@infradead.org \
--cc=vincent.guittot@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.