lkml.org 
[lkml]   [2022]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v4 01/25] rseq: Introduce feature size and alignment ELF auxiliary vector entries
From
On 2022-10-18 11:34, Florian Weimer wrote:
> * Mathieu Desnoyers:
>
>> If we extend struct rseq to a size that makes the compiler use an
>> alignment larger than 32 bytes in the future, and if the compiler uses
>> that larger alignment knowledge to issue instructions that require the
>> larger alignment, then it would be incorrect for user-space to
>> allocate the struct rseq on an alignment lower than the required
>> alignment.
>>
>> Indeed, on rseq registration, we have the following check:
>>
>> if (!IS_ALIGNED((unsigned long)rseq, __alignof__(*rseq))
>> [...]
>> return -EINVAL;
>>
>> Which would break if the size of struct rseq is large enough that the
>> alignment grows larger than 32 bytes.
>
> I never quite understood the reason for that check, it certainly made
> the glibc implementation more complicated. But to support variable
> sizes internally, we'll have to put in some extra effort anyway, so that
> it won't matter much in the end. As long as the required alignment
> isn't larger than the page size. 8-/

I don't expect it to grow so large.

There is one more reason why increasing the alignment of struct rseq may
become useful as the structure grows: it would guarantee that it fits in
a single lower level cache line as its size increases. It's not
something I expect would break if not properly aligned, but it's a nice
optimization.

I see two possible approaches here:

1) We expose the rseq alignment explicitly through auxv, and we can keep
the IS_ALIGNED validation on rseq registration. This "IS_ALIGNED" check
would probably have to be tweaked though, because if the registered
rseq size is 32, then an alignment of 32 is all we require. It's only if
the rseq_len is different from 32 that we need to validate that the
alignment matches the alignment of struct rseq.

2) We don't expose the rseq alignment through auxv, effectively fixing
it at 32. We would need to modify the IS_ALIGNED check on rseq
registration so it validates an alignment of 32 rather than using the
alignment of struct rseq.

>
>> You mentioned we could steal some high bits from AT_RSEQ_FEATURE_SIZE
>> to put the alignment. What is the issue with exposing an explicit
>> AT_RSEQ_ALIGN ? It's just a auxv entry, so I don't see it as a huge
>> performance concern to access 2 entries rather than one.
>
> I don't mind too much, we already have a large on-stack array in the
> loader so that we can decode the auxiliary vector without a humongous
> switch statement. But eventually that approach will stop working if the
> set of interesting AT_* values become too large and discontinuous.

OK. So I guess the main question here is whether we want fixed-32-bytes
alignment, or do we want to be able to increase the mandated alignment
in the future as struct rseq expands ?

The possible reasons for increasing the alignment over 32-bytes would be:

- Unforeseen compiler requirement on a structure alignment larger than
32-bytes as we extend the size of struct rseq.
- Optimization to fit within a single LLC cache line as struct rseq grows.

Thoughts ?

Thanks,

Mathieu

>
> Thanks,
> Florian
>

--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

\
 
 \ /
  Last update: 2022-10-18 21:01    [W:0.101 / U:0.324 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site