[PATCH 0/2] fix likely hint of sched_info_on()

[PATCH 0/2] fix likely hint of sched_info_on()

[Yi Wang]

When make defconfig, CONFIG_SCHEDSTATS is set to be y, so
sched_info_on() is 'likely' to be true. However, some functions
invoke this function with unlikely hint or use no hint. Let's
fix this.

Yi Wang (2):
  kvm: x86: add likely to sched_info_on()
  sched: fix unlikely use of sched_info_on()

 arch/x86/kvm/cpuid.c | 2 +-
 kernel/sched/stats.h | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] kvm: x86: add likely to sched_info_on()

[Yi Wang]

The condition to test is likely() to be true when make defconfig.
Add the hint.

Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
---
 arch/x86/kvm/cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 4992e7c..64fff41 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -642,7 +642,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 			     (1 << KVM_FEATURE_ASYNC_PF_VMEXIT) |
 			     (1 << KVM_FEATURE_PV_SEND_IPI);
 
-		if (sched_info_on())
+		if (likely(sched_info_on()))
 			entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
 
 		entry->ebx = 0;
-- 
1.8.3.1

Re: [PATCH 1/2] kvm: x86: add likely to sched_info_on()

[Paolo Bonzini]

On 04/07/19 13:46, Yi Wang wrote:
> The condition to test is likely() to be true when make defconfig.
> Add the hint.
> 
> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> ---
>  arch/x86/kvm/cpuid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 4992e7c..64fff41 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -642,7 +642,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  			     (1 << KVM_FEATURE_ASYNC_PF_VMEXIT) |
>  			     (1 << KVM_FEATURE_PV_SEND_IPI);
>  
> -		if (sched_info_on())
> +		if (likely(sched_info_on()))
>  			entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);

This is not a fast path, so adding likely/unlikely is unnecessary.

Paolo

>  		entry->ebx = 0;
> 

[PATCH 2/2] sched: fix unlikely use of sched_info_on()

[Yi Wang]

sched_info_on() is called with unlikely hint, however, the test
is to be true when make defconfig. So replace unlikely with
likely.

Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
---
 kernel/sched/stats.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/stats.h b/kernel/sched/stats.h
index aa0de24..b7a7c4d 100644
--- a/kernel/sched/stats.h
+++ b/kernel/sched/stats.h
@@ -157,7 +157,7 @@ static inline void sched_info_dequeued(struct rq *rq, struct task_struct *t)
 {
 	unsigned long long now = rq_clock(rq), delta = 0;
 
-	if (unlikely(sched_info_on()))
+	if (likely(sched_info_on()))
 		if (t->sched_info.last_queued)
 			delta = now - t->sched_info.last_queued;
 	sched_info_reset_dequeued(t);
@@ -192,7 +192,7 @@ static void sched_info_arrive(struct rq *rq, struct task_struct *t)
  */
 static inline void sched_info_queued(struct rq *rq, struct task_struct *t)
 {
-	if (unlikely(sched_info_on())) {
+	if (likely(sched_info_on())) {
 		if (!t->sched_info.last_queued)
 			t->sched_info.last_queued = rq_clock(rq);
 	}
@@ -239,7 +239,7 @@ static inline void sched_info_depart(struct rq *rq, struct task_struct *t)
 static inline void
 sched_info_switch(struct rq *rq, struct task_struct *prev, struct task_struct *next)
 {
-	if (unlikely(sched_info_on()))
+	if (likely(sched_info_on()))
 		__sched_info_switch(rq, prev, next);
 }
 
-- 
1.8.3.1

Re: [PATCH 0/2] fix likely hint of sched_info_on()

[Peter Zijlstra]

On Thu, Jul 04, 2019 at 07:46:13PM +0800, Yi Wang wrote:
> When make defconfig, CONFIG_SCHEDSTATS is set to be y, so
> sched_info_on() is 'likely' to be true. However, some functions
> invoke this function with unlikely hint or use no hint. Let's
> fix this.

How about remove the hint entirely? likely(1) is as rediculous as
unlikely(1), a constant is a constant and no amount of hinting should
make the compiler do anything else.

And if you want to retain the hint for the TASK_DELAY_ACCT nonsense,
stick it there.

Also, fix the lack of { } while you're there.