From: Ingo Molnar <mingo@elte.hu>
To: Alexey Dobriyan <adobriyan@sw.ru>
Cc: torvalds@osdl.org, linux-kernel@vger.kernel.org,
Jan Engelhardt <jengelh@computergmbh.de>,
Joe Perches <joe@perches.com>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH] De-constify sched.h
Date: Fri, 26 Oct 2007 19:32:46 +0200 [thread overview]
Message-ID: <20071026173246.GA7117@elte.hu> (raw)
In-Reply-To: <20071026081721.GA6244@localhost.sw.ru>
* Alexey Dobriyan <adobriyan@sw.ru> wrote:
> Linus, please revert commit a8972ccf00b7184a743eb6cd9bc7f3443357910c aka
> "sched: constify sched.h" or apply the following patch instead.
>
> [PATCH] De-constify sched.h
firstly, thank you for not Cc:-ing me. I learned about this revert only
once it was done deal in Linus' tree. You objected to the patch
originally when it was submitted to lkml, and plausible arguments were
presented against your (bogus) objection:
http://article.gmane.org/gmane.linux.kernel.janitors/14623
in that thread, two months ago, you essentially conceded Jan
Engelhardt's argument by not replying to his points, so i kept Joe's
patch. And now you continue this "discussion" by asking for a revert and
not Cc:-ing me, Joe or Jan - which is quite sneaky.
> 1) Patch doesn't change any code here, so gcc is already smart enough
> to "feel" constness in such simple functions.
the const markers indeed had no real purpose in terms of code generation
(gcc can figure it out whether something is modified by an inline
function), but they had a documentation/intent purpose, and they are
plausible if a non-inlined function uses a const task struct in the
future. For example any of these functions could be un-inlined and could
use (internally) one of the remaining inlines - in that case code
generation gets better from the constifying as well. There are also a
good deal of helper functions around task struct which could be marked
with const.
we do have other inline functions in include/linux/*.h with 'const'
arguments, so marking arguments with const is not without precedent and
this was pointed out to you in the original discussion.
> 2) There is no such thing as const task_struct. Anyone who think
> otherwise deserves compiler warning.
-ENOPARSE. 'const struct task_struct *' says that the task struct is not
supposed to be modified within that (inline) function. That _does_ make
sense.
so unless i'm missing something, your request for revert was pretty
rude, technically incorrect and you also tried to circumvent the normal
course of discussion. I have no strong feelings either way technically
(the patch is borderline - we dont actively pass around const
task_struct pointers at the moment - but we could start doing so, if we
had the constification), but i do have strong feelings against the kind
of behavior you showed here.
Ingo
prev parent reply other threads:[~2007-10-26 17:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-10-26 8:17 [PATCH] De-constify sched.h Alexey Dobriyan
2007-10-26 9:42 ` David Rientjes
2007-10-26 9:50 ` David Miller
2007-10-26 17:32 ` Ingo Molnar [this message]
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=20071026173246.GA7117@elte.hu \
--to=mingo@elte.hu \
--cc=adobriyan@sw.ru \
--cc=davem@davemloft.net \
--cc=jengelh@computergmbh.de \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@osdl.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox