Re: [PATCH] KVM: arm64: Select SCHED_INFO before SCHEDSTATS

[Steven Price]

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@xxxxxxxxxx>
>> ---
>> Â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<-----