linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Peter Anvin <hpa@zytor.com>,
	Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
	"the arch/x86 maintainers" <x86@kernel.org>
Subject: Re: [PATCH] x86_64: uninline TASK_SIZE
Date: Mon, 22 Apr 2019 15:40:00 -0700	[thread overview]
Message-ID: <CAHk-=wijYb1Ci2Sx2xxggxdqHuMT4HJ5WtW+eGJP-+Fa9b1YtA@mail.gmail.com> (raw)
In-Reply-To: <20190421160600.GA31092@avx2>

On Sun, Apr 21, 2019 at 9:06 AM Alexey Dobriyan <adobriyan@gmail.com> wrote:
>
> TASK_SIZE macro is quite deceptive: it looks like a constant but in fact
> compiles to 50+ bytes.

Honestly, if you are interested in improving TASK_SIZE, I'd really
like to see you try to go even further than this.

TASK_SIZE _used_ to just be a fixed constant, which is why it has that
name and why the usage patterns are what they are.

But since that isn't true any more, I'd much rather fix the _name_,
and I'd much rather fix the nasty complex hidden behavior, rather than
just keep the name and keep the behavior, but turning it from an
inline macro to a function call.

And as Ingo points out, we should be able to just make it a field of
its own, instead of that complex dance of TIF_ADDR32 etc.

However, I think it would be better if that field would be in "struct
mm_struct" instead of Ingo's suggestion of the thread. Because while
it's currently a per-thread flag, I think it is only set by execve(),
so it always ends up being the same per-mm. No?

Also, we could/should just make the existing *users* of TASK_SIZE know
that it's no longer a simple constant, so all those functions that use
it many times could just do

        unsigned long task_size = TASK_SIZE;

rather than re-compute it multiple times like they do now.

In fact, making it a function call in many ways makes things *worse*,
although maybe we could at least mark the function "pure" so that gcc
would be able to cache the end result. But that would actually be
wrong for the sequences that maybe do change the thread flags, so I
hate that idea too.

Much better to just cache it explicitly in the cases where we see that
it's currently generating bad code.

                Linus

      parent reply	other threads:[~2019-04-22 22:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-21 16:06 [PATCH] x86_64: uninline TASK_SIZE Alexey Dobriyan
2019-04-21 18:28 ` Ingo Molnar
2019-04-21 20:07   ` hpa
2019-04-21 21:10     ` Alexey Dobriyan
2019-04-22 10:34       ` Ingo Molnar
2019-04-22 14:30         ` Andy Lutomirski
2019-04-22 22:09           ` Alexey Dobriyan
2019-04-23  0:54             ` Andy Lutomirski
2019-04-23 11:12               ` Ingo Molnar
2019-04-23 17:21                 ` Andy Lutomirski
2019-04-24 10:38                   ` Ingo Molnar
2019-04-22 22:04         ` Alexey Dobriyan
2019-04-21 22:07   ` [PATCH v2] " Alexey Dobriyan
2019-04-22 22:40 ` Linus Torvalds [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='CAHk-=wijYb1Ci2Sx2xxggxdqHuMT4HJ5WtW+eGJP-+Fa9b1YtA@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=adobriyan@gmail.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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;
as well as URLs for NNTP newsgroup(s).