Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 28 Apr 2020 08:33:13 -0400 (EDT)
From:   Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To:     Florian Weimer <fw@deneb.enyo.de>
Cc:     Michael Kerrisk <mtk.manpages@gmail.com>,
        libc-alpha <libc-alpha@sourceware.org>,
        carlos <carlos@redhat.com>, Rich Felker <dalias@libc.org>,
        linux-api <linux-api@vger.kernel.org>,
        Boqun Feng <boqun.feng@gmail.com>,
        Will Deacon <will.deacon@arm.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Ben Maurer <bmaurer@fb.com>, Dave Watson <davejwatson@fb.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Paul <paulmck@linux.vnet.ibm.com>, Paul Turner <pjt@google.com>,
        Joseph Myers <joseph@codesourcery.com>,
        Szabolcs Nagy <szabolcs.nagy@arm.com>
Message-ID: <1080028389.72414.1588077193438.JavaMail.zimbra@efficios.com>
In-Reply-To: <87ftcnrf7d.fsf@mid.deneb.enyo.de>
References: <20200326155633.18236-1-mathieu.desnoyers@efficios.com> <20200326155633.18236-6-mathieu.desnoyers@efficios.com> <87ees9z417.fsf@mid.deneb.enyo.de> <284293396.70630.1588005648556.JavaMail.zimbra@efficios.com> <87zhawvphv.fsf@mid.deneb.enyo.de> <2102127737.70791.1588008377292.JavaMail.zimbra@efficios.com> <87ftcnrf7d.fsf@mid.deneb.enyo.de>
Subject: Re: [PATCH glibc 5/9] glibc: Perform rseq(2) registration at C
 startup and thread creation (v17)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Thread-Topic: glibc: Perform rseq(2) registration at C startup and thread creation (v17)
Thread-Index: z7tk/3iPPlCh9cj1VVKnVHBj0mM1xA==
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

----- On Apr 28, 2020, at 8:02 AM, Florian Weimer fw@deneb.enyo.de wrote:

> * Mathieu Desnoyers:
>=20
>>>>>> +/* struct rseq is aligned on 4 * 8 bytes to ensure it is always
>>>>>> +   contained within a single cache-line.
>>>>>> +
>>>>>> +   A single struct rseq per thread is allowed.  */
>>>>>> +struct rseq
>>>>>> +  {
>>>>>> +    /* Restartable sequences cpu_id_start field. Updated by the
>>>>>> +       kernel. Read by user-space with single-copy atomicity
>>>>>> +       semantics. This field should only be read by the thread whic=
h
>>>>>> +       registered this data structure. Aligned on 32-bit. Always
>>>>>=20
>>>>> What does =E2=80=9CAligned on 32-bit=E2=80=9D mean in this context?  =
Do you mean to
>>>>> reference 32-*byte* alignment here?
>>>>
>>>> No. I really mean 32-bit (4-byte). Being aligned on 32-byte guarantees=
 that
>>>> this field is aligned at least on 4-byte. This is required by single-c=
opy
>>>> atomicity semantics.
>>>>
>>>> Should I update this comment to state "Aligned on 4-byte" instead ?
>>>=20
>>> I think this is implied by all Linux ABIs.  And the explicit alignment
>>> specification for struct rseq makes the alignment 32 bytes.
>>
>> Unless a structure ends up being packed, which is of course not the case
>> here.
>>
>> I would prefer to keep the comment about 32-bit alignment requirement on
>> the specific fields, because the motivation for alignment requirement is
>> much more strict for fields (correctness) than the motivation for alignm=
ent
>> of the structure (performance).
>=20
> But the correctness is already enforced by the compiler, so I fail to
> see point of mentioning this in the comment.
>=20
> Anyway, I don't want to make a big deal of it.  Please leave it in if
> you think it is ehlpful.

I would prefer to leave it in, just to make the requirements plain clear in
case those structures are allocated on the heap (for instance).

>=20
>> x32 should not be an issue as explained above, so I'm very open to
>> add this "uptr" for user-space only.
>=20
> Okay, then please use anonymous unions and structs as necessary, to
> ensure that the uptr field can be reached on all platforms in the same
> way.

OK, will do!

One issue I'm currently facing when running "make check": because nptl/tst-=
rseq-nptl.c
uses pthread_cancel(), I run into an Abort with:

libgcc_s.so.1 must be installed for pthread_cancel to work
Didn't expect signal from child: got `Aborted'

So far I've tested the rest of that file with a patch on top which disables=
 the use of
pthread_cancel (), but I'd really like to give it a full coverage before se=
nding this out.
In https://sourceware.org/glibc/wiki/Testing/Builds there is a section abou=
t
"Building glibc with intent to install" which describes that libgcc must be=
 copied
manually. My use-case is that I just want to run "make check" in the build =
directory
and make sure it finds the libgcc it needs to succeed using pthread_cancel =
().
How can I achieve this ?

Thanks,

Mathieu


--=20
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com