From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: "Joel Fernandes, Google" <joel@joelfernandes.org>,
Chris Kennelly <ckennelly@google.com>
Cc: Paul Turner <pjt@google.com>, Florian Weimer <fweimer@redhat.com>,
Carlos O'Donell <codonell@redhat.com>,
libc-alpha <libc-alpha@sourceware.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
paulmck <paulmck@kernel.org>, Boqun Feng <boqun.feng@gmail.com>,
Brian Geffon <bgeffon@google.com>
Subject: Re: Rseq registration: Google tcmalloc vs glibc
Date: Wed, 26 Feb 2020 16:51:09 -0500 (EST) [thread overview]
Message-ID: <416869220.9190.1582753869096.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <CAEXW_YRT7AjaJs7mPyNd=J6fhBicYwGbQMK2Senwm3cBhFvWPw@mail.gmail.com>
----- On Feb 25, 2020, at 10:24 PM, Joel Fernandes, Google joel@joelfernandes.org wrote:
[..]
>
> Chris, Brian, is there any other concern to upgrading of tcmalloc
> version in ChromeOS? I believe there was some concern about detection
> of rseq kernel support. A quick look at tcmalloc shows it does not do
> such detection, but I can stand corrected. One more thing, currently
> tcmalloc does not use rseq on ARM. If I recall, ARM does have rseq
> support as well. So we ought to enable it for that arch as well if
> possible. Why not enable it on all arches and then dynamically detect
> at runtime if needed support is available?
Please allow me to raise a concern with respect to the implementation
of the SlowFence() function in tcmalloc/internal/percpu.cc. It uses
sched_setaffinity to move the thread around to each CPU part of the
cpu mask.
There are a couple of corner-cases where I think it can malfunction:
- Interaction with concurrent sched_setaffinity invoked by an external
manager process: If an external manager process attempts to limit this
thread's ability to run onto specific CPUs, either before the thread
starts or concurrently while the thread executes, I suspect the
SlowFence() algorithm will simply handle errors while trying to set
affinity by skipping CPUs, which results in a skipped rseq fence,
which in turn can cause corruption.
The comments in this function state:
// If we can't pin ourselves there, then no one else can run there, so
// that's fine.
But AFAIU the thread's cpu affinity is a per-thread attribute, so saying
that no other thread from the same process can run there seems wrong. What
am I missing ? Maybe it is a difference between cpusets and sched_setaffinity ?
The code below opens /proc/self/cpuset to deal with concurrent affinity
updates by cpuset seems to rely on CONFIG_CPUSETS=y, and does not seem to
take into account CPU affinity changes through sched_setaffinity.
Moreover, reading through the comments there, depending on internal kernel
synchronization implementation details for dealing with concurrent cpuset
updates seems very fragile. Those details about internal locking of
cpuset.cpus within the kernel should not be expected to be ABI.
- Interaction with CPU hotplug. If a target CPU is unplugged and plugged
again (offline, then online) concurrently, this algorithm may skip that
CPU and thus skip a rseq fence, which can also cause corruption.
Those limitations of sched_setaffinity() are the reasons why I have
proposed a new "pin_on_cpu()" system call [1]. Feedback in that area
is very welcome.
Thanks,
Mathieu
[1] https://lore.kernel.org/r/20200121160312.26545-1-mathieu.desnoyers@efficios.com
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
next prev parent reply other threads:[~2020-02-26 21:51 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-20 21:33 Rseq registration: Google tcmalloc vs glibc Mathieu Desnoyers
[not found] ` <CAEE+ybmTzNoBc9YmK2j48MKtSoPHC_1Mgr+ojPpiOTTc+4E=9g@mail.gmail.com>
2020-02-21 13:07 ` Florian Weimer
2020-02-21 15:49 ` Joel Fernandes
2020-02-21 16:13 ` Mathieu Desnoyers
2020-02-26 3:24 ` Joel Fernandes
2020-02-26 3:38 ` Chris Kennelly
2020-02-26 17:01 ` Mathieu Desnoyers
2020-02-26 17:27 ` Chris Kennelly
2020-02-26 18:56 ` Mathieu Desnoyers
2020-02-26 19:12 ` Chris Kennelly
2020-02-26 21:21 ` Mathieu Desnoyers
2020-02-27 10:18 ` Szabolcs Nagy
2020-02-27 10:32 ` Florian Weimer
2020-02-26 21:51 ` Mathieu Desnoyers [this message]
2020-02-27 21:11 ` Paul E. McKenney
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
List information: https://www.gnu.org/software/libc/involved.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=416869220.9190.1582753869096.JavaMail.zimbra@efficios.com \
--to=mathieu.desnoyers@efficios.com \
--cc=bgeffon@google.com \
--cc=boqun.feng@gmail.com \
--cc=ckennelly@google.com \
--cc=codonell@redhat.com \
--cc=fweimer@redhat.com \
--cc=joel@joelfernandes.org \
--cc=libc-alpha@sourceware.org \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=pjt@google.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).