linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Carlos O'Donell <carlos@redhat.com>
To: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	paulmck <paulmck@linux.ibm.com>,
	Boqun Feng <boqun.feng@gmail.com>,
	"H. Peter Anvin" <hpa@zytor.com>, Paul Turner <pjt@google.com>,
	linux-api <linux-api@vger.kernel.org>,
	Christian Brauner <christian.brauner@ubuntu.com>,
	Florian Weimer <fw@deneb.enyo.de>,
	Chris Kennelly <ckennelly@google.com>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [RFC PATCH 0/4] rseq: Introduce extensible struct rseq
Date: Thu, 16 Jul 2020 09:39:26 -0400	[thread overview]
Message-ID: <9f747c44-be54-c547-7e4c-a978499f06c0@redhat.com> (raw)
In-Reply-To: <1190016739.14119.1594818179014.JavaMail.zimbra@efficios.com>

On 7/15/20 9:02 AM, Mathieu Desnoyers wrote:
> At this point, the main question I would like answered is whether
> it would be acceptable to increase the size and alignment of
> the __rseq_abi symbol (which will be exposed by glibc) between
> e.g. glibc 2.32 and 2.33. If it's not possible, then we can
> find other solutions, for instance using an indirection with
> a pointer to an extended structure, but this appears to be
> slightly less efficient.

The answer is always a soft "maybe" because it depends exactly
on how we do it and what consequences we are willing to accept
in the design.

For example, static applications that call dlopen will fail if
we increase the alignment beyond 32 because we had to special
case this scenario. Why did we have to special case it? Because
the "static" part of the runtime needs to create the initial
thread's static TLS space, and since it doesn't know apriori
what will be loaded in the shared library, it needs to make a
"best guess" at the alignment requirement at startup.
We need to discuss this and agree that it's OK. We already want
to deprecate dynamic loading from static applications, so this
may not be a problem in general, but I hope you see my point.
That there are corner cases to be considered and ironed out.

I want to see a detailed design document explaining the various
compatibility issues and how we solve them along with the way
the extension mechanism would work and how it would be compliant
with C/C++ language rules in userspace without adding undue burden
of potentially having to use atomic instructions all the time.
This includes discussing how the headers change. We should also
talk out the options for symbol versioning and their consequences.
  
I haven't seen enough details, and there isn't really enough
time to discuss this. I think it is *great* that we are discussing
it, but it's safest if we revert rseq, finish the discussion,
and then finalize the inclusion for 2.33 with these details
ironed out.

I feel like we've made all the technical process we need to actually
include rseq in glibc, but this discussion, and the google example
(even if it doesn't match our use case) shows that if we spend another
month hammering out the extension details could yield something we
can use for years to come while we work out other details e.g. cpu_opv.

I can set aside time in the next month to write up such a document
and discuss these issues with you and Florian. The text would form
even more of the language we'd have to include in the man page for
the feature.

In the meantime I think we should revert rseq in glibc and take
our time to hash this out without the looming deadline of August 1st
for the ABI going out the door.

I know this is disappointing, but I think in a month you'll look
back at this, we'll have Fedora Rawhide using the new extensible
version (and you'll be able to point people at that), and we'll
only be 5 months away from an official release with extensible
rseq.

Could you please respond to Florian's request to revert here?
https://sourceware.org/pipermail/libc-alpha/2020-July/116368.html

I'm looking for a Signed-off-by from you that you're OK with
reverting.

-- 
Cheers,
Carlos.


  reply	other threads:[~2020-07-16 13:39 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-14  3:03 [RFC PATCH 0/4] rseq: Introduce extensible struct rseq Mathieu Desnoyers
2020-07-14  3:03 ` [RFC PATCH 1/4] selftests: rseq: Use fixed value as rseq_len parameter Mathieu Desnoyers
2020-07-14  3:03 ` [RFC PATCH 2/4] rseq: Allow extending struct rseq Mathieu Desnoyers
2020-07-14  9:58   ` Florian Weimer
2020-07-14 12:50     ` Mathieu Desnoyers
2020-07-14 13:00       ` Florian Weimer
2020-07-14 13:19         ` Mathieu Desnoyers
2020-07-14 21:30           ` Carlos O'Donell
2020-07-15 13:12             ` Mathieu Desnoyers
2020-07-15 13:22               ` Florian Weimer
2020-07-15 13:31                 ` Mathieu Desnoyers
2020-07-15 13:42                   ` Florian Weimer
2020-07-15 13:55                     ` Christian Brauner
2020-07-15 14:20                       ` Mathieu Desnoyers
2020-07-15 14:54                     ` Mathieu Desnoyers
2020-07-15 14:58                       ` Florian Weimer
2020-07-15 15:26                         ` Mathieu Desnoyers
2020-07-14 17:24   ` Peter Oskolkov
2020-07-14 17:43     ` Mathieu Desnoyers
2020-07-14 18:33       ` Peter Oskolkov
2020-07-15  2:34         ` Chris Kennelly
2020-07-15  6:31           ` Florian Weimer
2020-07-15 10:59             ` Christian Brauner
2020-07-15 14:38             ` Mathieu Desnoyers
2020-07-15 14:50           ` Mathieu Desnoyers
2020-07-15 11:38   ` Christian Brauner
2020-07-15 12:33     ` Christian Brauner
2020-07-15 15:10       ` Mathieu Desnoyers
2020-07-15 15:33         ` Christian Brauner
2020-07-14  3:03 ` [RFC PATCH 3/4] selftests: rseq: define __rseq_abi with extensible size Mathieu Desnoyers
2020-07-14  3:03 ` [RFC PATCH 4/4] selftests: rseq: print rseq extensible size in basic test Mathieu Desnoyers
2020-07-14 20:55 ` [RFC PATCH 0/4] rseq: Introduce extensible struct rseq Carlos O'Donell
2020-07-15 13:02   ` Mathieu Desnoyers
2020-07-16 13:39     ` Carlos O'Donell [this message]
2020-07-16 14:45       ` Mathieu Desnoyers
2020-07-15 15:12   ` Florian Weimer
2020-07-15 15:32     ` Mathieu Desnoyers

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=9f747c44-be54-c547-7e4c-a978499f06c0@redhat.com \
    --to=carlos@redhat.com \
    --cc=boqun.feng@gmail.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=ckennelly@google.com \
    --cc=fw@deneb.enyo.de \
    --cc=hpa@zytor.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=paulmck@linux.ibm.com \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.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).