All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Philipp Klocke <Phil_K97@gmx.de>
Cc: sil2review@lists.osadl.org, kernelnewbies@kernelnewbies.org,
	llvmlinux@lists.linuxfoundation.org,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@redhat.com>,
	der.herr@hofr.at, lukas.bulwahn@gmail.com
Subject: Re: [PATCH] sched/fair: Change sched_feat(x) in !CONFIG_SCHED_DEBUG case
Date: Fri, 20 Apr 2018 18:51:39 +0200	[thread overview]
Message-ID: <20180420165139.GP4064@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <34572fee-36d0-36e1-ba6d-f098b145aba4@gmx.de>

On Fri, Apr 20, 2018 at 06:29:07PM +0200, Philipp Klocke wrote:
> The gain is stopping a warning that clutters the output log of clang.

Well, you should not be using clang anyway. It is known to miscompile
the kernel.

> To improve readability, one can drop the ifdef-structure and just keep
> the right shift version, like Nicholas suggested. This will have a (very
> small)
> impact on performance in CONFIG_SCHED_DEBUG case, but when
> debugging, performance is no problem anyways.

See that is two bad choices.

> > Also, if sysctl_sched_features is a constant, the both expressions
> > _should_ really result in a constant and clang should still warn about
> > it.
> No, because clang only warns if the constant is neither 1 nor 0.
> (These being the 'best' integer representations of booleans)

Then won't something like so work?

#define sched_feat(x) !!(sysctl_sched_features & (1UL << __SCHED_FEAT_##x))

That forces it into _Bool space (0/1) and per the above rule will no
longer warn.

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

WARNING: multiple messages have this Message-ID (diff)
From: peterz@infradead.org (Peter Zijlstra)
To: kernelnewbies@lists.kernelnewbies.org
Subject: [PATCH] sched/fair: Change sched_feat(x) in !CONFIG_SCHED_DEBUG case
Date: Fri, 20 Apr 2018 18:51:39 +0200	[thread overview]
Message-ID: <20180420165139.GP4064@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <34572fee-36d0-36e1-ba6d-f098b145aba4@gmx.de>

On Fri, Apr 20, 2018 at 06:29:07PM +0200, Philipp Klocke wrote:
> The gain is stopping a warning that clutters the output log of clang.

Well, you should not be using clang anyway. It is known to miscompile
the kernel.

> To improve readability, one can drop the ifdef-structure and just keep
> the right shift version, like Nicholas suggested. This will have a (very
> small)
> impact on performance in CONFIG_SCHED_DEBUG case, but when
> debugging, performance is no problem anyways.

See that is two bad choices.

> > Also, if sysctl_sched_features is a constant, the both expressions
> > _should_ really result in a constant and clang should still warn about
> > it.
> No, because clang only warns if the constant is neither 1 nor 0.
> (These being the 'best' integer representations of booleans)

Then won't something like so work?

#define sched_feat(x) !!(sysctl_sched_features & (1UL << __SCHED_FEAT_##x))

That forces it into _Bool space (0/1) and per the above rule will no
longer warn.

  reply	other threads:[~2018-04-20 16:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-16  8:54 [PATCH] sched/fair: Change sched_feat(x) in !CONFIG_SCHED_DEBUG case Philipp Klocke
2018-04-18 13:49 ` Nicholas Mc Guire
2018-04-18 13:49   ` Nicholas Mc Guire
2018-04-20  7:57 ` Peter Zijlstra
2018-04-20  7:57   ` Peter Zijlstra
2018-04-20 16:29   ` Philipp Klocke
2018-04-20 16:29     ` Philipp Klocke
2018-04-20 16:51     ` Peter Zijlstra [this message]
2018-04-20 16:51       ` Peter Zijlstra
2018-04-20 21:29       ` Lukas Bulwahn
2018-04-20 21:29         ` Lukas Bulwahn
2018-04-23  9:45         ` Peter Zijlstra
2018-04-23  9:45           ` Peter Zijlstra
2018-04-20 10:34 ` Peter Zijlstra
2018-04-20 10:34   ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180420165139.GP4064@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=Phil_K97@gmx.de \
    --cc=der.herr@hofr.at \
    --cc=kernelnewbies@kernelnewbies.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvmlinux@lists.linuxfoundation.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=mingo@redhat.com \
    --cc=sil2review@lists.osadl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.