* [PATCH] KVM: arm64: Select SCHED_INFO before SCHEDSTATS
@ 2019-10-23 3:22 Mao Wenan
2019-10-23 12:32 ` Steven Price
2019-10-23 16:51 ` Marc Zyngier
0 siblings, 2 replies; 6+ messages in thread
From: Mao Wenan @ 2019-10-23 3:22 UTC (permalink / raw)
To: maz, james.morse, julien.thierry.kdev, suzuki.poulose,
catalin.marinas, will, steven.price
Cc: Mao Wenan, kernel-janitors, kvmarm, linux-arm-kernel, linux-kernel
If KVM=y, it will select SCHEDSTATS, below erros can
be seen:
kernel/sched/stats.h: In function rq_sched_info_arrive:
kernel/sched/stats.h:12:20: error: struct sched_info
has no member named run_delay
rq->rq_sched_info.run_delay += delta;
^
kernel/sched/stats.h:13:20: error: struct sched_info
has no member named pcount
rq->rq_sched_info.pcount++;
^
kernel/sched/stats.h: In function rq_sched_info_dequeued:
kernel/sched/stats.h:31:20: error: struct sched_info has
no member named run_delay
rq->rq_sched_info.run_delay += delta;
These are because CONFIG_SCHED_INFO is not set, This patch
is to select SCHED_INFO before SCHEDSTATS.
Fixes: 8564d6372a7d ("KVM: arm64: Support stolen time reporting via shared structure")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
arch/arm64/kvm/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index d8b88e4..3c46eac 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -39,6 +39,7 @@ config KVM
select IRQ_BYPASS_MANAGER
select HAVE_KVM_IRQ_BYPASS
select HAVE_KVM_VCPU_RUN_PID_CHANGE
+ select SCHED_INFO
select SCHEDSTATS
---help---
Support hosting virtualized guest machines.
--
2.7.4
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: arm64: Select SCHED_INFO before SCHEDSTATS
2019-10-23 3:22 [PATCH] KVM: arm64: Select SCHED_INFO before SCHEDSTATS Mao Wenan
@ 2019-10-23 12:32 ` Steven Price
2019-10-23 16:51 ` Marc Zyngier
1 sibling, 0 replies; 6+ messages in thread
From: Steven Price @ 2019-10-23 12:32 UTC (permalink / raw)
To: Mao Wenan
Cc: maz, kernel-janitors, linux-kernel, linux-arm-kernel,
Catalin Marinas, will, kvmarm
On Wed, Oct 23, 2019 at 04:22:54AM +0100, Mao Wenan wrote:
> If KVM=y, it will select SCHEDSTATS, below erros can
> be seen:
> kernel/sched/stats.h: In function rq_sched_info_arrive:
> kernel/sched/stats.h:12:20: error: struct sched_info
> has no member named run_delay
> rq->rq_sched_info.run_delay += delta;
> ^
> kernel/sched/stats.h:13:20: error: struct sched_info
> has no member named pcount
> rq->rq_sched_info.pcount++;
> ^
> kernel/sched/stats.h: In function rq_sched_info_dequeued:
> kernel/sched/stats.h:31:20: error: struct sched_info has
> no member named run_delay
> rq->rq_sched_info.run_delay += delta;
>
> These are because CONFIG_SCHED_INFO is not set, This patch
> is to select SCHED_INFO before SCHEDSTATS.
It looks like I didn't spot this because when DEBUG_KERNEL is enabled
then KVM selects SCHEDSTATS, which selects SCHED_INFO. Thanks for
spotting this.
>
> Fixes: 8564d6372a7d ("KVM: arm64: Support stolen time reporting via shared structure")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
Reviewed-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm64/kvm/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index d8b88e4..3c46eac 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -39,6 +39,7 @@ config KVM
> select IRQ_BYPASS_MANAGER
> select HAVE_KVM_IRQ_BYPASS
> select HAVE_KVM_VCPU_RUN_PID_CHANGE
> + select SCHED_INFO
> select SCHEDSTATS
> ---help---
> Support hosting virtualized guest machines.
> --
> 2.7.4
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: arm64: Select SCHED_INFO before SCHEDSTATS
2019-10-23 3:22 [PATCH] KVM: arm64: Select SCHED_INFO before SCHEDSTATS Mao Wenan
2019-10-23 12:32 ` Steven Price
@ 2019-10-23 16:51 ` Marc Zyngier
2019-10-24 11:22 ` Steven Price
1 sibling, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2019-10-23 16:51 UTC (permalink / raw)
To: Mao Wenan, steven.price
Cc: catalin.marinas, kernel-janitors, linux-kernel, will, kvmarm,
linux-arm-kernel
On 2019-10-23 04:22, Mao Wenan wrote:
> If KVM=y, it will select SCHEDSTATS, below erros can
> be seen:
> kernel/sched/stats.h: In function rq_sched_info_arrive:
> kernel/sched/stats.h:12:20: error: struct sched_info
> has no member named run_delay
> rq->rq_sched_info.run_delay += delta;
> ^
> kernel/sched/stats.h:13:20: error: struct sched_info
> has no member named pcount
> rq->rq_sched_info.pcount++;
> ^
> kernel/sched/stats.h: In function rq_sched_info_dequeued:
> kernel/sched/stats.h:31:20: error: struct sched_info has
> no member named run_delay
> rq->rq_sched_info.run_delay += delta;
>
> These are because CONFIG_SCHED_INFO is not set, This patch
> is to select SCHED_INFO before SCHEDSTATS.
>
> Fixes: 8564d6372a7d ("KVM: arm64: Support stolen time reporting via
> shared structure")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
> arch/arm64/kvm/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index d8b88e4..3c46eac 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -39,6 +39,7 @@ config KVM
> select IRQ_BYPASS_MANAGER
> select HAVE_KVM_IRQ_BYPASS
> select HAVE_KVM_VCPU_RUN_PID_CHANGE
> + select SCHED_INFO
> select SCHEDSTATS
> ---help---
> Support hosting virtualized guest machines.
SCHEDSTATS is really an odd choice. Here's what I get after disabling
DEBUG_KERNEL (from defconfig):
WARNING: unmet direct dependencies detected for SCHEDSTATS
Depends on [n]: DEBUG_KERNEL [=n] && PROC_FS [=y]
Selected by [y]:
- KVM [=y] && VIRTUALIZATION [=y] && OF [=y]
WARNING: unmet direct dependencies detected for SCHEDSTATS
Depends on [n]: DEBUG_KERNEL [=n] && PROC_FS [=y]
Selected by [y]:
- KVM [=y] && VIRTUALIZATION [=y] && OF [=y]
WARNING: unmet direct dependencies detected for SCHEDSTATS
Depends on [n]: DEBUG_KERNEL [=n] && PROC_FS [=y]
Selected by [y]:
- KVM [=y] && VIRTUALIZATION [=y] && OF [=y]
So clearly SCHEDSTATS isn't meant to be selected on its own.
We can either just select SCHED_INFO (which *nobody else does*), or go
the full x86 way which selects TASK_DELAY_ACCT (and thus depends on
NET && MULTIUSER). My gut feeling is that we shouldn't deviate too much
from x86...
Thoughts?
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: arm64: Select SCHED_INFO before SCHEDSTATS
2019-10-23 16:51 ` Marc Zyngier
@ 2019-10-24 11:22 ` Steven Price
2019-10-24 13:19 ` Marc Zyngier
0 siblings, 1 reply; 6+ messages in thread
From: Steven Price @ 2019-10-24 11:22 UTC (permalink / raw)
To: Marc Zyngier, Mao Wenan
Cc: catalin.marinas, kernel-janitors, linux-kernel, will, kvmarm,
linux-arm-kernel
On 23/10/2019 17:51, Marc Zyngier wrote:
> On 2019-10-23 04:22, Mao Wenan wrote:
>> If KVM=y, it will select SCHEDSTATS, below erros can
>> be seen:
>> kernel/sched/stats.h: In function rq_sched_info_arrive:
>> kernel/sched/stats.h:12:20: error: struct sched_info
>> has no member named run_delay
>> rq->rq_sched_info.run_delay += delta;
>> ^
>> kernel/sched/stats.h:13:20: error: struct sched_info
>> has no member named pcount
>> rq->rq_sched_info.pcount++;
>> ^
>> kernel/sched/stats.h: In function rq_sched_info_dequeued:
>> kernel/sched/stats.h:31:20: error: struct sched_info has
>> no member named run_delay
>> rq->rq_sched_info.run_delay += delta;
>>
>> These are because CONFIG_SCHED_INFO is not set, This patch
>> is to select SCHED_INFO before SCHEDSTATS.
>>
>> Fixes: 8564d6372a7d ("KVM: arm64: Support stolen time reporting via
>> shared structure")
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>> arch/arm64/kvm/Kconfig | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
>> index d8b88e4..3c46eac 100644
>> --- a/arch/arm64/kvm/Kconfig
>> +++ b/arch/arm64/kvm/Kconfig
>> @@ -39,6 +39,7 @@ config KVM
>> select IRQ_BYPASS_MANAGER
>> select HAVE_KVM_IRQ_BYPASS
>> select HAVE_KVM_VCPU_RUN_PID_CHANGE
>> + select SCHED_INFO
>> select SCHEDSTATS
>> ---help---
>> Support hosting virtualized guest machines.
>
> SCHEDSTATS is really an odd choice. Here's what I get after disabling
> DEBUG_KERNEL (from defconfig):
>
> WARNING: unmet direct dependencies detected for SCHEDSTATS
> Depends on [n]: DEBUG_KERNEL [=n] && PROC_FS [=y]
> Selected by [y]:
> - KVM [=y] && VIRTUALIZATION [=y] && OF [=y]
>
> WARNING: unmet direct dependencies detected for SCHEDSTATS
> Depends on [n]: DEBUG_KERNEL [=n] && PROC_FS [=y]
> Selected by [y]:
> - KVM [=y] && VIRTUALIZATION [=y] && OF [=y]
>
> WARNING: unmet direct dependencies detected for SCHEDSTATS
> Depends on [n]: DEBUG_KERNEL [=n] && PROC_FS [=y]
> Selected by [y]:
> - KVM [=y] && VIRTUALIZATION [=y] && OF [=y]
>
> So clearly SCHEDSTATS isn't meant to be selected on its own.
>
> We can either just select SCHED_INFO (which *nobody else does*), or go
> the full x86 way which selects TASK_DELAY_ACCT (and thus depends on
> NET && MULTIUSER). My gut feeling is that we shouldn't deviate too much
> from x86...
>
> Thoughts?
I suspect you're right - TASK_DELAY_ACCT seems to be the closest to what
we need. SCHEDSTATS has the "advantage" of forcing sched_info_on() to
return true - preventing it from being disabled. But we clearly don't
want to require CONFIG_DEBUG_KERNEL for CONFIG_KVM.
The next best is CONFIG_TASK_DELAY_ACCT which enables sched_info_on()
unless "nodelayacct" is specified on the cmdline. It seems reasonable
that the cmdline option might break stolen time.
So let's just copy x86:
-----8<-----
From 915893f5c57241cc29d90769b3f720a6135277d7 Mon Sep 17 00:00:00 2001
From: Steven Price <steven.price@arm.com>
Date: Thu, 24 Oct 2019 12:14:36 +0100
Subject: [PATCH] KVM: arm64: Select TASK_DELAY_ACCT rather than SCHEDSTATS
SCHEDSTATS requires DEBUG_KERNEL (and PROC_FS) and therefore isn't a
good choice for enabling the scheduling statistics required for stolen
time.
Instead match the x86 configuration and select TASK_DELAY_ACCT. This
adds the dependencies of NET && MULTIUSER for arm64 KVM.
Suggested-by: Marc Zyngier <maz@kernel.org>
Fixes: 8564d6372a7d ("KVM: arm64: Support stolen time reporting via shared structure")
Signed-off-by: Steven Price <steven.price@arm.com>
---
arch/arm64/kvm/Kconfig | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index d8b88e40d223..1ffb300e2d92 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -21,6 +21,8 @@ if VIRTUALIZATION
config KVM
bool "Kernel-based Virtual Machine (KVM) support"
depends on OF
+ # for TASKSTATS/TASK_DELAY_ACCT:
+ depends on NET && MULTIUSER
select MMU_NOTIFIER
select PREEMPT_NOTIFIERS
select HAVE_KVM_CPU_RELAX_INTERCEPT
@@ -39,7 +41,7 @@ config KVM
select IRQ_BYPASS_MANAGER
select HAVE_KVM_IRQ_BYPASS
select HAVE_KVM_VCPU_RUN_PID_CHANGE
- select SCHEDSTATS
+ select TASK_DELAY_ACCT
---help---
Support hosting virtualized guest machines.
We don't support KVM with 16K page tables yet, due to the multiple
--
2.20.1
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] KVM: arm64: Select SCHED_INFO before SCHEDSTATS
2019-10-24 11:22 ` Steven Price
@ 2019-10-24 13:19 ` Marc Zyngier
2019-10-24 13:31 ` [PATCH v2] KVM: arm64: Select TASK_DELAY_ACCT+TASKSTATS rather than SCHEDSTATS Steven Price
0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2019-10-24 13:19 UTC (permalink / raw)
To: Steven Price
Cc: catalin.marinas, kernel-janitors, Mao Wenan, linux-kernel, will,
kvmarm, linux-arm-kernel
On 2019-10-24 12:22, Steven Price wrote:
[...]
> From 915893f5c57241cc29d90769b3f720a6135277d7 Mon Sep 17 00:00:00
> 2001
> From: Steven Price <steven.price@arm.com>
> Date: Thu, 24 Oct 2019 12:14:36 +0100
> Subject: [PATCH] KVM: arm64: Select TASK_DELAY_ACCT rather than
> SCHEDSTATS
>
> SCHEDSTATS requires DEBUG_KERNEL (and PROC_FS) and therefore isn't a
> good choice for enabling the scheduling statistics required for
> stolen
> time.
>
> Instead match the x86 configuration and select TASK_DELAY_ACCT. This
> adds the dependencies of NET && MULTIUSER for arm64 KVM.
>
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Fixes: 8564d6372a7d ("KVM: arm64: Support stolen time reporting via
> shared structure")
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> arch/arm64/kvm/Kconfig | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
> index d8b88e40d223..1ffb300e2d92 100644
> --- a/arch/arm64/kvm/Kconfig
> +++ b/arch/arm64/kvm/Kconfig
> @@ -21,6 +21,8 @@ if VIRTUALIZATION
> config KVM
> bool "Kernel-based Virtual Machine (KVM) support"
> depends on OF
> + # for TASKSTATS/TASK_DELAY_ACCT:
> + depends on NET && MULTIUSER
> select MMU_NOTIFIER
> select PREEMPT_NOTIFIERS
> select HAVE_KVM_CPU_RELAX_INTERCEPT
> @@ -39,7 +41,7 @@ config KVM
> select IRQ_BYPASS_MANAGER
> select HAVE_KVM_IRQ_BYPASS
> select HAVE_KVM_VCPU_RUN_PID_CHANGE
> - select SCHEDSTATS
> + select TASK_DELAY_ACCT
> ---help---
> Support hosting virtualized guest machines.
> We don't support KVM with 16K page tables yet, due to the
> multiple
Same issue as before: you have an implicit config symbol selection.
TASK_DELAY_ACCT depends on TASKSTATS (which is why you have this NET &&
MULTIUSER constraint).
You need to select both TASK_DELAY_ACCT and TASKSTATS, as the comment
you
add suggests.
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2] KVM: arm64: Select TASK_DELAY_ACCT+TASKSTATS rather than SCHEDSTATS
2019-10-24 13:19 ` Marc Zyngier
@ 2019-10-24 13:31 ` Steven Price
0 siblings, 0 replies; 6+ messages in thread
From: Steven Price @ 2019-10-24 13:31 UTC (permalink / raw)
To: Marc Zyngier
Cc: catalin.marinas, kernel-janitors, Mao Wenan, linux-kernel,
Steven Price, will, kvmarm, linux-arm-kernel
SCHEDSTATS requires DEBUG_KERNEL (and PROC_FS) and therefore isn't a
good choice for enabling the scheduling statistics required for stolen
time.
Instead match the x86 configuration and select TASK_DELAY_ACCT and
TASKSTATS. This adds the dependencies of NET && MULTIUSER for arm64 KVM.
Suggested-by: Marc Zyngier <maz@kernel.org>
Fixes: 8564d6372a7d ("KVM: arm64: Support stolen time reporting via shared structure")
Signed-off-by: Steven Price <steven.price@arm.com>
---
Let's try again! Somehow I'd got it into my head that TASK_DELAY_ACCT
selected TASKSTATS not depended on. Even though I'd managed to get it
right in the comment!
arch/arm64/kvm/Kconfig | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index d8b88e40d223..a475c68cbfec 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -21,6 +21,8 @@ if VIRTUALIZATION
config KVM
bool "Kernel-based Virtual Machine (KVM) support"
depends on OF
+ # for TASKSTATS/TASK_DELAY_ACCT:
+ depends on NET && MULTIUSER
select MMU_NOTIFIER
select PREEMPT_NOTIFIERS
select HAVE_KVM_CPU_RELAX_INTERCEPT
@@ -39,7 +41,8 @@ config KVM
select IRQ_BYPASS_MANAGER
select HAVE_KVM_IRQ_BYPASS
select HAVE_KVM_VCPU_RUN_PID_CHANGE
- select SCHEDSTATS
+ select TASKSTATS
+ select TASK_DELAY_ACCT
---help---
Support hosting virtualized guest machines.
We don't support KVM with 16K page tables yet, due to the multiple
--
2.20.1
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-24 13:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-23 3:22 [PATCH] KVM: arm64: Select SCHED_INFO before SCHEDSTATS Mao Wenan
2019-10-23 12:32 ` Steven Price
2019-10-23 16:51 ` Marc Zyngier
2019-10-24 11:22 ` Steven Price
2019-10-24 13:19 ` Marc Zyngier
2019-10-24 13:31 ` [PATCH v2] KVM: arm64: Select TASK_DELAY_ACCT+TASKSTATS rather than SCHEDSTATS Steven Price
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).