| Message ID | 20170719125310.2487451-7-arnd@arndb.de |
|---|---|
| State | Accepted |
| Commit | c2ce3f5d89d57301e2756ac325fe2ebc33bfec30 |
| Headers | show |
| Series | x86: randconfig warning fixes | expand |
2017-07-19 14:53+0200, Arnd Bergmann: > KVM tries to select 'TASKSTATS', which had additional dependencies: > > warning: (KVM) selects TASKSTATS which has unmet direct dependencies (NET && MULTIUSER) > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- Hm, do you know why Kconfig warns instead of propagating the dependencies? > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > @@ -22,7 +22,7 @@ config KVM > # for TASKSTATS/TASK_DELAY_ACCT: > - depends on NET > + depends on NET && MULTIUSER The current condition goes halfway to nowhere, so the patch is definitely an improvement, even if the result is not good ... Applied, thanks.
On Wed, Jul 19, 2017 at 4:11 PM, Radim Krčmář <rkrcmar@redhat.com> wrote: > 2017-07-19 14:53+0200, Arnd Bergmann: >> KVM tries to select 'TASKSTATS', which had additional dependencies: >> >> warning: (KVM) selects TASKSTATS which has unmet direct dependencies (NET && MULTIUSER) >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- > > Hm, do you know why Kconfig warns instead of propagating the > dependencies? Kconfig propagates 'depends on' dependencies, but cannot turn a 'select' into 'depends on', as those two mean different things. Another solution to the problem would be to use 'depends on TASKSTATS'. Generally speaking, using 'select' to turn on a user-visible option is a bad idea, but blindly turning those 'select' into 'depends on' is also dangerous, as it can break configurations of existing users that would here end up with neither TASKSTATS nor KVM after a 'make oldconfig'. Arnd
2017-07-19 16:18+0200, Arnd Bergmann: > On Wed, Jul 19, 2017 at 4:11 PM, Radim Krčmář <rkrcmar@redhat.com> wrote: > > 2017-07-19 14:53+0200, Arnd Bergmann: > >> KVM tries to select 'TASKSTATS', which had additional dependencies: > >> > >> warning: (KVM) selects TASKSTATS which has unmet direct dependencies (NET && MULTIUSER) > >> > >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > >> --- > > > > Hm, do you know why Kconfig warns instead of propagating the > > dependencies? > > Kconfig propagates 'depends on' dependencies, but cannot turn a 'select' > into 'depends on', as those two mean different things. > > Another solution to the problem would be to use 'depends on TASKSTATS'. Good point, 'select' seems misused here. There is no reason to depend on TASKSTATS (nor NET+MULTIUSER), we only suggest to enable it with KVM. KVM uses sched_info_on() to handle any any possible resulting configuration, c9aaa8957f20 ("KVM: Steal time implementation"). KVM would work as intended if 'select' would not enable the option if its dependencies failed (instead of unconditionally forcing the option). Is the preferred way to encode it: 'default y if KVM' in config TASK_DELAY_ACCT (that adds a non-local and enigmatic dependency and also needlessly expands the possible configuration space) or 'select TASKSTATS if NET && MULTIUSER' in config KVM (that is going to break when dependencies of TASKSTATS change again) ? thanks.
On 19/07/2017 18:13, Radim Krčmář wrote: > Good point, 'select' seems misused here. > > There is no reason to depend on TASKSTATS (nor NET+MULTIUSER), we only > suggest to enable it with KVM. KVM uses sched_info_on() to handle any > any possible resulting configuration, c9aaa8957f20 ("KVM: Steal time > implementation"). > > KVM would work as intended if 'select' would not enable the option if > its dependencies failed (instead of unconditionally forcing the option). > > Is the preferred way to encode it: > > 'default y if KVM' in config TASK_DELAY_ACCT > (that adds a non-local and enigmatic dependency and also needlessly > expands the possible configuration space) > > or > > 'select TASKSTATS if NET && MULTIUSER' in config KVM > (that is going to break when dependencies of TASKSTATS change again) > > ? I think the former is the closest to what the user actually wants, and it would let us clean up arch/x86/kvm/Kconfig. However it should be "default y if KVM && X86'. Maybe there is room for a new operator "suggest Y" which, when added inside "config X", operates as if "config Y" had a "default y if X". In this case, kvm could do - depends on NET && MULTIUSER - select TASKSTATS + suggest TASKSTATS Thanks, Paolo
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index 760433b2574a..2688c7dc5323 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -22,7 +22,7 @@ config KVM depends on HAVE_KVM depends on HIGH_RES_TIMERS # for TASKSTATS/TASK_DELAY_ACCT: - depends on NET + depends on NET && MULTIUSER select PREEMPT_NOTIFIERS select MMU_NOTIFIER select ANON_INODES
KVM tries to select 'TASKSTATS', which had additional dependencies: warning: (KVM) selects TASKSTATS which has unmet direct dependencies (NET && MULTIUSER) Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/x86/kvm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.9.0