2024-02-08 20:18:26

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH v2 0/5] sched/vtime: vtime.h headers cleanup

Hi All,

I kept all tags on reveiwed patches.

v2:

- patch 4: commit message reworded (Heiko)
- patch 5: vtime.h is removed from Kbuild scripts (PowerPC only) (Heiko)

v1:

Please find a small cleanup to vtime_task_switch() wiring.
I split it into smaller patches to allow separate PowerPC
vs s390 reviews. Otherwise patches 2+3 and 4+5 could have
been merged.

I tested it on s390 and compile-tested it on 32- and 64-bit
PowerPC and few other major architectures only, but it is
only of concern for CONFIG_VIRT_CPU_ACCOUNTING_NATIVE-capable
ones (AFAICT).

Thanks!

Alexander Gordeev (5):
sched/vtime: remove confusing arch_vtime_task_switch() declaration
sched/vtime: get rid of generic vtime_task_switch() implementation
s390/vtime: remove unused __ARCH_HAS_VTIME_TASK_SWITCH leftover
s390/irq,nmi: include <asm/vtime.h> header directly
sched/vtime: do not include <asm/vtime.h> header

arch/powerpc/include/asm/Kbuild | 1 -
arch/powerpc/include/asm/cputime.h | 13 -------------
arch/powerpc/kernel/time.c | 22 ++++++++++++++++++++++
arch/s390/include/asm/vtime.h | 2 --
arch/s390/kernel/irq.c | 1 +
arch/s390/kernel/nmi.c | 1 +
include/asm-generic/vtime.h | 1 -
include/linux/vtime.h | 5 -----
kernel/sched/cputime.c | 13 -------------
9 files changed, 24 insertions(+), 35 deletions(-)
delete mode 100644 include/asm-generic/vtime.h

--
2.40.1



2024-02-08 20:18:38

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH v2 1/5] sched/vtime: remove confusing arch_vtime_task_switch() declaration

Callback arch_vtime_task_switch() is only defined when
CONFIG_VIRT_CPU_ACCOUNTING_NATIVE is selected. Yet, the
function prototype forward declaration is present for
CONFIG_VIRT_CPU_ACCOUNTING_GEN variant. Remove it.

Reviewed-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
include/linux/vtime.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/include/linux/vtime.h b/include/linux/vtime.h
index 3684487d01e1..593466ceebed 100644
--- a/include/linux/vtime.h
+++ b/include/linux/vtime.h
@@ -18,7 +18,6 @@ extern void vtime_account_idle(struct task_struct *tsk);
#endif /* !CONFIG_VIRT_CPU_ACCOUNTING */

#ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN
-extern void arch_vtime_task_switch(struct task_struct *tsk);
extern void vtime_user_enter(struct task_struct *tsk);
extern void vtime_user_exit(struct task_struct *tsk);
extern void vtime_guest_enter(struct task_struct *tsk);
--
2.40.1


2024-02-08 20:54:49

by Alexander Gordeev

[permalink] [raw]
Subject: [PATCH v2 5/5] sched/vtime: do not include <asm/vtime.h> header

There is no architecture-specific code or data left
that generic <linux/vtime.h> needs to know about.
Thus, avoid the inclusion of <asm/vtime.h> header.

Reviewed-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Alexander Gordeev <[email protected]>
---
arch/powerpc/include/asm/Kbuild | 1 -
include/asm-generic/vtime.h | 1 -
include/linux/vtime.h | 4 ----
3 files changed, 6 deletions(-)
delete mode 100644 include/asm-generic/vtime.h

diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index 61a8d5555cd7..e5fdc336c9b2 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -6,5 +6,4 @@ generic-y += agp.h
generic-y += kvm_types.h
generic-y += mcs_spinlock.h
generic-y += qrwlock.h
-generic-y += vtime.h
generic-y += early_ioremap.h
diff --git a/include/asm-generic/vtime.h b/include/asm-generic/vtime.h
deleted file mode 100644
index b1a49677fe25..000000000000
--- a/include/asm-generic/vtime.h
+++ /dev/null
@@ -1 +0,0 @@
-/* no content, but patch(1) dislikes empty files */
diff --git a/include/linux/vtime.h b/include/linux/vtime.h
index 593466ceebed..29dd5b91dd7d 100644
--- a/include/linux/vtime.h
+++ b/include/linux/vtime.h
@@ -5,10 +5,6 @@
#include <linux/context_tracking_state.h>
#include <linux/sched.h>

-#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
-#include <asm/vtime.h>
-#endif
-
/*
* Common vtime APIs
*/
--
2.40.1


2024-02-09 03:38:19

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] sched/vtime: do not include <asm/vtime.h> header

On Fri Feb 9, 2024 at 6:15 AM AEST, Alexander Gordeev wrote:
> There is no architecture-specific code or data left
> that generic <linux/vtime.h> needs to know about.
> Thus, avoid the inclusion of <asm/vtime.h> header.

Nice cleanup!

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

>
> Reviewed-by: Frederic Weisbecker <[email protected]>
> Signed-off-by: Alexander Gordeev <[email protected]>
> ---
> arch/powerpc/include/asm/Kbuild | 1 -
> include/asm-generic/vtime.h | 1 -
> include/linux/vtime.h | 4 ----
> 3 files changed, 6 deletions(-)
> delete mode 100644 include/asm-generic/vtime.h
>
> diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
> index 61a8d5555cd7..e5fdc336c9b2 100644
> --- a/arch/powerpc/include/asm/Kbuild
> +++ b/arch/powerpc/include/asm/Kbuild
> @@ -6,5 +6,4 @@ generic-y += agp.h
> generic-y += kvm_types.h
> generic-y += mcs_spinlock.h
> generic-y += qrwlock.h
> -generic-y += vtime.h
> generic-y += early_ioremap.h
> diff --git a/include/asm-generic/vtime.h b/include/asm-generic/vtime.h
> deleted file mode 100644
> index b1a49677fe25..000000000000
> --- a/include/asm-generic/vtime.h
> +++ /dev/null
> @@ -1 +0,0 @@
> -/* no content, but patch(1) dislikes empty files */
> diff --git a/include/linux/vtime.h b/include/linux/vtime.h
> index 593466ceebed..29dd5b91dd7d 100644
> --- a/include/linux/vtime.h
> +++ b/include/linux/vtime.h
> @@ -5,10 +5,6 @@
> #include <linux/context_tracking_state.h>
> #include <linux/sched.h>
>
> -#ifdef CONFIG_VIRT_CPU_ACCOUNTING_NATIVE
> -#include <asm/vtime.h>
> -#endif
> -
> /*
> * Common vtime APIs
> */


2024-02-09 03:41:58

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] sched/vtime: remove confusing arch_vtime_task_switch() declaration

On Fri Feb 9, 2024 at 6:15 AM AEST, Alexander Gordeev wrote:
> Callback arch_vtime_task_switch() is only defined when
> CONFIG_VIRT_CPU_ACCOUNTING_NATIVE is selected. Yet, the
> function prototype forward declaration is present for
> CONFIG_VIRT_CPU_ACCOUNTING_GEN variant. Remove it.
>

And powerpc arch_vtime_task_switch is static inline too,
so this just confuses things.

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

> Reviewed-by: Frederic Weisbecker <[email protected]>
> Signed-off-by: Alexander Gordeev <[email protected]>
> ---
> include/linux/vtime.h | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/include/linux/vtime.h b/include/linux/vtime.h
> index 3684487d01e1..593466ceebed 100644
> --- a/include/linux/vtime.h
> +++ b/include/linux/vtime.h
> @@ -18,7 +18,6 @@ extern void vtime_account_idle(struct task_struct *tsk);
> #endif /* !CONFIG_VIRT_CPU_ACCOUNTING */
>
> #ifdef CONFIG_VIRT_CPU_ACCOUNTING_GEN
> -extern void arch_vtime_task_switch(struct task_struct *tsk);
> extern void vtime_user_enter(struct task_struct *tsk);
> extern void vtime_user_exit(struct task_struct *tsk);
> extern void vtime_guest_enter(struct task_struct *tsk);