Messages in this thread | | | From | Florian Weimer <> | Subject | Re: [PATCH glibc 5/9] glibc: Perform rseq(2) registration at C startup and thread creation (v17) | Date | Tue, 28 Apr 2020 14:02:14 +0200 |
| |
* 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.
> 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.
| |