All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kvm: remove dependence on delay-accounting
@ 2012-01-14 16:24 Konstantin Khlebnikov
  2012-01-14 16:30 ` [PATCH v2] " Konstantin Khlebnikov
  0 siblings, 1 reply; 7+ messages in thread
From: Konstantin Khlebnikov @ 2012-01-14 16:24 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Marcelo Tosatti, Avi Kivity
  Cc: linux-kernel, kvm

KVM selects delay-accounting only to get sched-info for steal-time accounting.
Meanwhile delay-accounting can be disabled by boot option. This is ridiculous.

This patch adds internal boolean option CONFIG_TASK_SCHED_INFO to enable only
task->sched_info and its collecting inside scheduler.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
 arch/x86/kvm/Kconfig  |    5 +----
 include/linux/sched.h |    6 ++----
 init/Kconfig          |    7 +++++++
 kernel/sched/core.c   |    2 +-
 kernel/sched/stats.h  |    4 ++--
 lib/Kconfig.debug     |    1 +
 6 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 1a7fe86..2e27d8c 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -22,8 +22,6 @@ config KVM
 	depends on HAVE_KVM
 	# for device assignment:
 	depends on PCI
-	# for TASKSTATS/TASK_DELAY_ACCT:
-	depends on NET
 	select PREEMPT_NOTIFIERS
 	select MMU_NOTIFIER
 	select ANON_INODES
@@ -33,8 +31,7 @@ config KVM
 	select KVM_ASYNC_PF
 	select USER_RETURN_NOTIFIER
 	select KVM_MMIO
-	select TASKSTATS
-	select TASK_DELAY_ACCT
+	select TAKS_SCHED_INFO
 	select PERF_EVENTS
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 868cb83..dd5bf78 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -734,7 +734,6 @@ extern struct user_struct root_user;
 
 struct backing_dev_info;
 
-#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
 struct sched_info {
 	/* cumulative counters */
 	unsigned long pcount;	      /* # of times run on this cpu */
@@ -744,7 +743,6 @@ struct sched_info {
 	unsigned long long last_arrival,/* when we last ran on a cpu */
 			   last_queued;	/* when we were last queued to run */
 };
-#endif /* defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT) */
 
 #ifdef CONFIG_TASK_DELAY_ACCT
 struct task_delay_info {
@@ -782,7 +780,7 @@ struct task_delay_info {
 
 static inline int sched_info_on(void)
 {
-#ifdef CONFIG_SCHEDSTATS
+#if IS_ENABLED(CONFIG_SCHEDSTATS) || IS_ENABLED(CONFIG_KVM)
 	return 1;
 #elif defined(CONFIG_TASK_DELAY_ACCT)
 	extern int delayacct_on;
@@ -1285,7 +1283,7 @@ struct task_struct {
 	struct rt_mutex *rcu_boost_mutex;
 #endif /* #ifdef CONFIG_RCU_BOOST */
 
-#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
+#ifdef CONFIG_TASK_SCHED_INFO
 	struct sched_info sched_info;
 #endif
 
diff --git a/init/Kconfig b/init/Kconfig
index 6ac2236..ee0dd4b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -318,6 +318,7 @@ config TASKSTATS
 config TASK_DELAY_ACCT
 	bool "Enable per-task delay accounting (EXPERIMENTAL)"
 	depends on TASKSTATS
+	select TASK_SCHED_INFO
 	help
 	  Collect information on time spent by a task waiting for system
 	  resources like cpu, synchronous block I/O completion and swapping
@@ -860,6 +861,12 @@ config SCHED_AUTOGROUP
 	  desktop applications.  Task group autogeneration is currently based
 	  upon task session.
 
+#
+# collect per-task scheduler statistics
+#
+config TASK_SCHED_INFO
+	bool
+
 config MM_OWNER
 	bool
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fd7b25e..bf5616d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1758,7 +1758,7 @@ void sched_fork(struct task_struct *p)
 	set_task_cpu(p, cpu);
 	raw_spin_unlock_irqrestore(&p->pi_lock, flags);
 
-#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
+#ifdef CONFIG_TASK_SCHED_INFO
 	if (likely(sched_info_on()))
 		memset(&p->sched_info, 0, sizeof(p->sched_info));
 #endif
diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
index 2ef90a5..2322b86 100644
--- a/kernel/sched/stats.h
+++ b/kernel/sched/stats.h
@@ -47,7 +47,7 @@ rq_sched_info_depart(struct rq *rq, unsigned long long delta)
 # define schedstat_set(var, val)	do { } while (0)
 #endif
 
-#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
+#ifdef CONFIG_TASK_SCHED_INFO
 static inline void sched_info_reset_dequeued(struct task_struct *t)
 {
 	t->sched_info.last_queued = 0;
@@ -153,7 +153,7 @@ sched_info_switch(struct task_struct *prev, struct task_struct *next)
 #define sched_info_reset_dequeued(t)	do { } while (0)
 #define sched_info_dequeued(t)			do { } while (0)
 #define sched_info_switch(t, next)		do { } while (0)
-#endif /* CONFIG_SCHEDSTATS || CONFIG_TASK_DELAY_ACCT */
+#endif /* CONFIG_TASK_SCHED_INFO */
 
 /*
  * The following are functions that support scheduler-internal time accounting.
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 82928f5..f2e9ee3 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -290,6 +290,7 @@ config SCHED_DEBUG
 config SCHEDSTATS
 	bool "Collect scheduler statistics"
 	depends on DEBUG_KERNEL && PROC_FS
+	select TASK_SCHED_INFO
 	help
 	  If you say Y here, additional code will be inserted into the
 	  scheduler and related routines to collect statistics about


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2] kvm: remove dependence on delay-accounting
  2012-01-14 16:24 [PATCH] kvm: remove dependence on delay-accounting Konstantin Khlebnikov
@ 2012-01-14 16:30 ` Konstantin Khlebnikov
  2012-01-16 11:14   ` Marcelo Tosatti
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Konstantin Khlebnikov @ 2012-01-14 16:30 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Marcelo Tosatti, Avi Kivity
  Cc: linux-kernel, kvm

KVM selects delay-accounting only to get sched-info for steal-time accounting.
Meanwhile delay-accounting can be disabled by boot option. This is ridiculous.

This patch adds internal boolean option CONFIG_TASK_SCHED_INFO to enable only
task->sched_info and its collecting inside scheduler.

v2:
* stupid misprint fixed

Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
 arch/x86/kvm/Kconfig  |    5 +----
 include/linux/sched.h |    6 ++----
 init/Kconfig          |    7 +++++++
 kernel/sched/core.c   |    2 +-
 kernel/sched/stats.h  |    4 ++--
 lib/Kconfig.debug     |    1 +
 6 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 1a7fe86..e3952e8 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -22,8 +22,6 @@ config KVM
 	depends on HAVE_KVM
 	# for device assignment:
 	depends on PCI
-	# for TASKSTATS/TASK_DELAY_ACCT:
-	depends on NET
 	select PREEMPT_NOTIFIERS
 	select MMU_NOTIFIER
 	select ANON_INODES
@@ -33,8 +31,7 @@ config KVM
 	select KVM_ASYNC_PF
 	select USER_RETURN_NOTIFIER
 	select KVM_MMIO
-	select TASKSTATS
-	select TASK_DELAY_ACCT
+	select TASK_SCHED_INFO
 	select PERF_EVENTS
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 868cb83..dd5bf78 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -734,7 +734,6 @@ extern struct user_struct root_user;
 
 struct backing_dev_info;
 
-#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
 struct sched_info {
 	/* cumulative counters */
 	unsigned long pcount;	      /* # of times run on this cpu */
@@ -744,7 +743,6 @@ struct sched_info {
 	unsigned long long last_arrival,/* when we last ran on a cpu */
 			   last_queued;	/* when we were last queued to run */
 };
-#endif /* defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT) */
 
 #ifdef CONFIG_TASK_DELAY_ACCT
 struct task_delay_info {
@@ -782,7 +780,7 @@ struct task_delay_info {
 
 static inline int sched_info_on(void)
 {
-#ifdef CONFIG_SCHEDSTATS
+#if IS_ENABLED(CONFIG_SCHEDSTATS) || IS_ENABLED(CONFIG_KVM)
 	return 1;
 #elif defined(CONFIG_TASK_DELAY_ACCT)
 	extern int delayacct_on;
@@ -1285,7 +1283,7 @@ struct task_struct {
 	struct rt_mutex *rcu_boost_mutex;
 #endif /* #ifdef CONFIG_RCU_BOOST */
 
-#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
+#ifdef CONFIG_TASK_SCHED_INFO
 	struct sched_info sched_info;
 #endif
 
diff --git a/init/Kconfig b/init/Kconfig
index 6ac2236..ee0dd4b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -318,6 +318,7 @@ config TASKSTATS
 config TASK_DELAY_ACCT
 	bool "Enable per-task delay accounting (EXPERIMENTAL)"
 	depends on TASKSTATS
+	select TASK_SCHED_INFO
 	help
 	  Collect information on time spent by a task waiting for system
 	  resources like cpu, synchronous block I/O completion and swapping
@@ -860,6 +861,12 @@ config SCHED_AUTOGROUP
 	  desktop applications.  Task group autogeneration is currently based
 	  upon task session.
 
+#
+# collect per-task scheduler statistics
+#
+config TASK_SCHED_INFO
+	bool
+
 config MM_OWNER
 	bool
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fd7b25e..bf5616d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1758,7 +1758,7 @@ void sched_fork(struct task_struct *p)
 	set_task_cpu(p, cpu);
 	raw_spin_unlock_irqrestore(&p->pi_lock, flags);
 
-#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
+#ifdef CONFIG_TASK_SCHED_INFO
 	if (likely(sched_info_on()))
 		memset(&p->sched_info, 0, sizeof(p->sched_info));
 #endif
diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
index 2ef90a5..2322b86 100644
--- a/kernel/sched/stats.h
+++ b/kernel/sched/stats.h
@@ -47,7 +47,7 @@ rq_sched_info_depart(struct rq *rq, unsigned long long delta)
 # define schedstat_set(var, val)	do { } while (0)
 #endif
 
-#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
+#ifdef CONFIG_TASK_SCHED_INFO
 static inline void sched_info_reset_dequeued(struct task_struct *t)
 {
 	t->sched_info.last_queued = 0;
@@ -153,7 +153,7 @@ sched_info_switch(struct task_struct *prev, struct task_struct *next)
 #define sched_info_reset_dequeued(t)	do { } while (0)
 #define sched_info_dequeued(t)			do { } while (0)
 #define sched_info_switch(t, next)		do { } while (0)
-#endif /* CONFIG_SCHEDSTATS || CONFIG_TASK_DELAY_ACCT */
+#endif /* CONFIG_TASK_SCHED_INFO */
 
 /*
  * The following are functions that support scheduler-internal time accounting.
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 82928f5..f2e9ee3 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -290,6 +290,7 @@ config SCHED_DEBUG
 config SCHEDSTATS
 	bool "Collect scheduler statistics"
 	depends on DEBUG_KERNEL && PROC_FS
+	select TASK_SCHED_INFO
 	help
 	  If you say Y here, additional code will be inserted into the
 	  scheduler and related routines to collect statistics about


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] kvm: remove dependence on delay-accounting
  2012-01-14 16:30 ` [PATCH v2] " Konstantin Khlebnikov
@ 2012-01-16 11:14   ` Marcelo Tosatti
  2012-01-16 13:42     ` Konstantin Khlebnikov
  2012-01-16 13:33   ` Peter Zijlstra
  2012-01-16 13:54   ` [PATCH] sched: remove task-sched-info dynamic disabler Konstantin Khlebnikov
  2 siblings, 1 reply; 7+ messages in thread
From: Marcelo Tosatti @ 2012-01-16 11:14 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Peter Zijlstra, Ingo Molnar, Avi Kivity, linux-kernel, kvm

On Sat, Jan 14, 2012 at 08:30:51PM +0400, Konstantin Khlebnikov wrote:
> KVM selects delay-accounting only to get sched-info for steal-time accounting.
> Meanwhile delay-accounting can be disabled by boot option. This is ridiculous.
> 
> This patch adds internal boolean option CONFIG_TASK_SCHED_INFO to enable only
> task->sched_info and its collecting inside scheduler.
> 
> v2:
> * stupid misprint fixed
> 
> Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
> ---
>  arch/x86/kvm/Kconfig  |    5 +----
>  include/linux/sched.h |    6 ++----
>  init/Kconfig          |    7 +++++++
>  kernel/sched/core.c   |    2 +-
>  kernel/sched/stats.h  |    4 ++--
>  lib/Kconfig.debug     |    1 +
>  6 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 1a7fe86..e3952e8 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -22,8 +22,6 @@ config KVM
>  	depends on HAVE_KVM
>  	# for device assignment:
>  	depends on PCI
> -	# for TASKSTATS/TASK_DELAY_ACCT:
> -	depends on NET
>  	select PREEMPT_NOTIFIERS
>  	select MMU_NOTIFIER
>  	select ANON_INODES
> @@ -33,8 +31,7 @@ config KVM
>  	select KVM_ASYNC_PF
>  	select USER_RETURN_NOTIFIER
>  	select KVM_MMIO
> -	select TASKSTATS
> -	select TASK_DELAY_ACCT
> +	select TASK_SCHED_INFO
>  	select PERF_EVENTS
>  	---help---
>  	  Support hosting fully virtualized guest machines using hardware
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 868cb83..dd5bf78 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -734,7 +734,6 @@ extern struct user_struct root_user;
>  
>  struct backing_dev_info;
>  
> -#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
>  struct sched_info {
>  	/* cumulative counters */
>  	unsigned long pcount;	      /* # of times run on this cpu */
> @@ -744,7 +743,6 @@ struct sched_info {
>  	unsigned long long last_arrival,/* when we last ran on a cpu */
>  			   last_queued;	/* when we were last queued to run */
>  };
> -#endif /* defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT) */
>  
>  #ifdef CONFIG_TASK_DELAY_ACCT
>  struct task_delay_info {
> @@ -782,7 +780,7 @@ struct task_delay_info {
>  
>  static inline int sched_info_on(void)
>  {
> -#ifdef CONFIG_SCHEDSTATS
> +#if IS_ENABLED(CONFIG_SCHEDSTATS) || IS_ENABLED(CONFIG_KVM)
>  	return 1;

CONFIG_TASK_SCHED_INFO?


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] kvm: remove dependence on delay-accounting
  2012-01-14 16:30 ` [PATCH v2] " Konstantin Khlebnikov
  2012-01-16 11:14   ` Marcelo Tosatti
@ 2012-01-16 13:33   ` Peter Zijlstra
  2012-01-16 14:05     ` Konstantin Khlebnikov
  2012-01-16 13:54   ` [PATCH] sched: remove task-sched-info dynamic disabler Konstantin Khlebnikov
  2 siblings, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2012-01-16 13:33 UTC (permalink / raw)
  To: Konstantin Khlebnikov
  Cc: Ingo Molnar, Marcelo Tosatti, Avi Kivity, linux-kernel, kvm

On Sat, 2012-01-14 at 20:30 +0400, Konstantin Khlebnikov wrote:
> KVM selects delay-accounting only to get sched-info for steal-time accounting.
> Meanwhile delay-accounting can be disabled by boot option. This is ridiculous.
> 
> This patch adds internal boolean option CONFIG_TASK_SCHED_INFO to enable only
> task->sched_info and its collecting inside scheduler.

Urgh, more stupid config knobs, we should be removing them, not adding
moar.

> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 868cb83..dd5bf78 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -734,7 +734,6 @@ extern struct user_struct root_user;
>  
>  struct backing_dev_info;
>  
> -#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
>  struct sched_info {
>  	/* cumulative counters */
>  	unsigned long pcount;	      /* # of times run on this cpu */
> @@ -744,7 +743,6 @@ struct sched_info {
>  	unsigned long long last_arrival,/* when we last ran on a cpu */
>  			   last_queued;	/* when we were last queued to run */
>  };
> -#endif /* defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT) */

Not having that structure helps with compile errors.

>  #ifdef CONFIG_TASK_DELAY_ACCT
>  struct task_delay_info {
> @@ -782,7 +780,7 @@ struct task_delay_info {
>  
>  static inline int sched_info_on(void)
>  {
> -#ifdef CONFIG_SCHEDSTATS
> +#if IS_ENABLED(CONFIG_SCHEDSTATS) || IS_ENABLED(CONFIG_KVM)

WTF is IS_ENABLED and why do you use it?


Not much like this stuff.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] kvm: remove dependence on delay-accounting
  2012-01-16 11:14   ` Marcelo Tosatti
@ 2012-01-16 13:42     ` Konstantin Khlebnikov
  0 siblings, 0 replies; 7+ messages in thread
From: Konstantin Khlebnikov @ 2012-01-16 13:42 UTC (permalink / raw)
  To: Marcelo Tosatti
  Cc: Peter Zijlstra, Ingo Molnar, Avi Kivity, linux-kernel, kvm

Marcelo Tosatti wrote:
> On Sat, Jan 14, 2012 at 08:30:51PM +0400, Konstantin Khlebnikov wrote:
>> KVM selects delay-accounting only to get sched-info for steal-time accounting.
>> Meanwhile delay-accounting can be disabled by boot option. This is ridiculous.
>>
>> This patch adds internal boolean option CONFIG_TASK_SCHED_INFO to enable only
>> task->sched_info and its collecting inside scheduler.
>>

<cut>

>>   static inline int sched_info_on(void)
>>   {
>> -#ifdef CONFIG_SCHEDSTATS
>> +#if IS_ENABLED(CONFIG_SCHEDSTATS) || IS_ENABLED(CONFIG_KVM)
>>   	return 1;
>
> CONFIG_TASK_SCHED_INFO?
>

It makes it equal to constant 1, because all its callers are
under #ifdef CONFIG_TASK_SCHED_INFO =)

its current code:

static inline int sched_info_on(void)
{
#ifdef CONFIG_SCHEDSTATS
	return 1;
#elif defined(CONFIG_TASK_DELAY_ACCT)
	extern int delayacct_on;
	return delayacct_on;
#else
	return 0;
#endif
}

CONFIG_SCHEDSTATS == debug option in lib/Kconfig.debug for /proc/schedstat
CONFIG_TASK_DELAY_ACCT == wierd net-link based statistics collecting tool

Thus, may be better to remove this function, because it can return 0
only if delay-accounting is compiled but disabled by boot option
(delayacct_on =1 by default since 2.6.18)

patch follows...


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] sched: remove task-sched-info dynamic disabler
  2012-01-14 16:30 ` [PATCH v2] " Konstantin Khlebnikov
  2012-01-16 11:14   ` Marcelo Tosatti
  2012-01-16 13:33   ` Peter Zijlstra
@ 2012-01-16 13:54   ` Konstantin Khlebnikov
  2 siblings, 0 replies; 7+ messages in thread
From: Konstantin Khlebnikov @ 2012-01-16 13:54 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Marcelo Tosatti, Avi Kivity
  Cc: linux-kernel, kvm

Currently per-task sched-info statistics can be disabled if delay-accounting is
disabled by boot option. There is no reason for it, sched-info is built-in
into task-struct, and its collecting does not add any extra atomic operations.
In other combinations it either not compiled or cannot be disabled.

This patch removes sched_info_on() and fixes all its users.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@openvz.org>
---
 arch/x86/kvm/cpuid.c  |    6 ++----
 arch/x86/kvm/x86.c    |    4 ----
 include/linux/sched.h |   12 ------------
 kernel/sched/core.c   |    3 +--
 kernel/sched/stats.h  |   18 +++++-------------
 5 files changed, 8 insertions(+), 35 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 89b02bf..12870d8 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -408,10 +408,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 			     (1 << KVM_FEATURE_NOP_IO_DELAY) |
 			     (1 << KVM_FEATURE_CLOCKSOURCE2) |
 			     (1 << KVM_FEATURE_ASYNC_PF) |
-			     (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
-
-		if (sched_info_on())
-			entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
+			     (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT) |
+			     (1 << KVM_FEATURE_STEAL_TIME);
 
 		entry->ebx = 0;
 		entry->ecx = 0;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 14d6cad..a60645c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1579,10 +1579,6 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
 			return 1;
 		break;
 	case MSR_KVM_STEAL_TIME:
-
-		if (unlikely(!sched_info_on()))
-			return 1;
-
 		if (data & KVM_STEAL_RESERVED_MASK)
 			return 1;
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index dd5bf78..eb8842e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -778,18 +778,6 @@ struct task_delay_info {
 };
 #endif	/* CONFIG_TASK_DELAY_ACCT */
 
-static inline int sched_info_on(void)
-{
-#if IS_ENABLED(CONFIG_SCHEDSTATS) || IS_ENABLED(CONFIG_KVM)
-	return 1;
-#elif defined(CONFIG_TASK_DELAY_ACCT)
-	extern int delayacct_on;
-	return delayacct_on;
-#else
-	return 0;
-#endif
-}
-
 enum cpu_idle_type {
 	CPU_IDLE,
 	CPU_NOT_IDLE,
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 0c59dcb..73acf77 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1759,8 +1759,7 @@ void sched_fork(struct task_struct *p)
 	raw_spin_unlock_irqrestore(&p->pi_lock, flags);
 
 #ifdef CONFIG_TASK_SCHED_INFO
-	if (likely(sched_info_on()))
-		memset(&p->sched_info, 0, sizeof(p->sched_info));
+	memset(&p->sched_info, 0, sizeof(p->sched_info));
 #endif
 #if defined(CONFIG_SMP)
 	p->on_cpu = 0;
diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
index 2322b86..e6363ed 100644
--- a/kernel/sched/stats.h
+++ b/kernel/sched/stats.h
@@ -63,9 +63,8 @@ static inline void sched_info_dequeued(struct task_struct *t)
 {
 	unsigned long long now = task_rq(t)->clock, delta = 0;
 
-	if (unlikely(sched_info_on()))
-		if (t->sched_info.last_queued)
-			delta = now - t->sched_info.last_queued;
+	if (t->sched_info.last_queued)
+		delta = now - t->sched_info.last_queued;
 	sched_info_reset_dequeued(t);
 	t->sched_info.run_delay += delta;
 
@@ -98,9 +97,8 @@ static void sched_info_arrive(struct task_struct *t)
  */
 static inline void sched_info_queued(struct task_struct *t)
 {
-	if (unlikely(sched_info_on()))
-		if (!t->sched_info.last_queued)
-			t->sched_info.last_queued = task_rq(t)->clock;
+	if (!t->sched_info.last_queued)
+		t->sched_info.last_queued = task_rq(t)->clock;
 }
 
 /*
@@ -127,7 +125,7 @@ static inline void sched_info_depart(struct task_struct *t)
  * the idle task.)  We are only called when prev != next.
  */
 static inline void
-__sched_info_switch(struct task_struct *prev, struct task_struct *next)
+sched_info_switch(struct task_struct *prev, struct task_struct *next)
 {
 	struct rq *rq = task_rq(prev);
 
@@ -142,12 +140,6 @@ __sched_info_switch(struct task_struct *prev, struct task_struct *next)
 	if (next != rq->idle)
 		sched_info_arrive(next);
 }
-static inline void
-sched_info_switch(struct task_struct *prev, struct task_struct *next)
-{
-	if (unlikely(sched_info_on()))
-		__sched_info_switch(prev, next);
-}
 #else
 #define sched_info_queued(t)			do { } while (0)
 #define sched_info_reset_dequeued(t)	do { } while (0)


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] kvm: remove dependence on delay-accounting
  2012-01-16 13:33   ` Peter Zijlstra
@ 2012-01-16 14:05     ` Konstantin Khlebnikov
  0 siblings, 0 replies; 7+ messages in thread
From: Konstantin Khlebnikov @ 2012-01-16 14:05 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Marcelo Tosatti, Avi Kivity, linux-kernel, kvm

Peter Zijlstra wrote:
> On Sat, 2012-01-14 at 20:30 +0400, Konstantin Khlebnikov wrote:
>> KVM selects delay-accounting only to get sched-info for steal-time accounting.
>> Meanwhile delay-accounting can be disabled by boot option. This is ridiculous.
>>
>> This patch adds internal boolean option CONFIG_TASK_SCHED_INFO to enable only
>> task->sched_info and its collecting inside scheduler.
>
> Urgh, more stupid config knobs, we should be removing them, not adding
> moar.

Unfortunately, removing task-delay-accounting is not the option =)

>
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index 868cb83..dd5bf78 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -734,7 +734,6 @@ extern struct user_struct root_user;
>>
>>   struct backing_dev_info;
>>
>> -#if defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT)
>>   struct sched_info {
>>   	/* cumulative counters */
>>   	unsigned long pcount;	      /* # of times run on this cpu */
>> @@ -744,7 +743,6 @@ struct sched_info {
>>   	unsigned long long last_arrival,/* when we last ran on a cpu */
>>   			   last_queued;	/* when we were last queued to run */
>>   };
>> -#endif /* defined(CONFIG_SCHEDSTATS) || defined(CONFIG_TASK_DELAY_ACCT) */
>
> Not having that structure helps with compile errors.

I don't think so, this only helps to catch useless declarations and definitions
in code and inside other structures.

>
>>   #ifdef CONFIG_TASK_DELAY_ACCT
>>   struct task_delay_info {
>> @@ -782,7 +780,7 @@ struct task_delay_info {
>>
>>   static inline int sched_info_on(void)
>>   {
>> -#ifdef CONFIG_SCHEDSTATS
>> +#if IS_ENABLED(CONFIG_SCHEDSTATS) || IS_ENABLED(CONFIG_KVM)
>
> WTF is IS_ENABLED and why do you use it?

IS_ENABLED(smth) == (defined(smth) || defined(smth_MODULE))
but using it for CONFIG_SCHEDSTATS is overkill, indeed.

>
>
> Not much like this stuff.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-01-16 14:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-01-14 16:24 [PATCH] kvm: remove dependence on delay-accounting Konstantin Khlebnikov
2012-01-14 16:30 ` [PATCH v2] " Konstantin Khlebnikov
2012-01-16 11:14   ` Marcelo Tosatti
2012-01-16 13:42     ` Konstantin Khlebnikov
2012-01-16 13:33   ` Peter Zijlstra
2012-01-16 14:05     ` Konstantin Khlebnikov
2012-01-16 13:54   ` [PATCH] sched: remove task-sched-info dynamic disabler Konstantin Khlebnikov

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.