----- On Apr 28, 2020, at 8:02 AM, Florian Weimer fw@xxxxxxxxxxxxx wrote:
> * Mathieu Desnoyers:
>
>>>>>> +/* 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 which
>>>>>> + registered this data structure. Aligned on 32-bit. Always
>>>>>
>>>>> What does “Aligned on 32-bit” 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-copy
>>>> atomicity semantics.
>>>>
>>>> Should I update this comment to state "Aligned on 4-byte" instead ?
>>>
>>> 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 alignment
>> of the structure (performance).
>
> But the correctness is already enforced by the compiler, so I fail to
> see point of mentioning this in the comment.
>
> 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).
>
>> x32 should not be an issue as explained above, so I'm very open to
>> add this "uptr" for user-space only.
>
> 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 sending this out.
In https://sourceware.org/glibc/wiki/Testing/Builds there is a section about
"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
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
