2019-07-04 11:48:57

by Yi Wang

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

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

2019-07-04 11:49:31

by Yi Wang

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

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 <[email protected]>
---
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

2019-07-04 11:50:36

by Yi Wang

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

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

Signed-off-by: Yi Wang <[email protected]>
---
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

2019-07-04 12:16:35

by Paolo Bonzini

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

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 <[email protected]>
> ---
> 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;
>

2019-07-04 12:20:12

by Peter Zijlstra

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

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.