Re: [PATCH 1/2] m68k: Use vDSO for thread pointer access
On 08/01/26 19:34, Stefan Reinauer wrote:
> Thank you for the detailed review, Adhemerval!
>
> On Thu, Jan 8, 2026 at 6:10 AM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 08/01/26 03:18, Stefan wrote:
>>> Optimize __m68k_read_tp() to read the thread pointer directly from
>>> the vDSO data page when available, avoiding the get_thread_area
>>> syscall overhead.
>>>
>>> The kernel maintains the thread pointer in a data page located at
>>> a fixed offset (PAGE_SIZE) before the vDSO ELF header. This value
>>> is updated on every context switch and set_thread_area() call.
>>>
>>> The implementation uses lazy initialization: on first call, it
>>> checks AT_SYSINFO_EHDR from auxv to locate the vDSO. If the vDSO
>>> is not available (older kernels), it falls back to the syscall.
>>>
>>
>> When was the m68k vDSO support added on Linus tree? I removed some
>> ancient code that was added based on a out-of-tree patches (commit
>> 112a0ae18b831bf31f44d81b82666980312511d6) and afaik m68k vDSO support
>> was never added on Linux.
>
> It hasn't been - these glibc patches depend on kernel patches that are
> currently under an initial review on linux-m68k. I should
> have made that dependency clear and included links to the kernel
> patches. The kernel patch series is at:
> https://marc.info/?l=linux-m68k&m=176785594629016&w=2
I had the impression this was the case, adding the kernel links along
with a [RFC] would give us more hint.
Usually when kernel patches are sent for review we assume they are
already in Linus tree.
But thanks to sent it prior is become kABI, it give us time to narrow
down possible issues.
>
>>> diff --git a/sysdeps/unix/sysv/linux/m68k/m68k-helpers.c b/sysdeps/unix/sysv/linux/m68k/m68k-helpers.c
>>> index 845aae51..1dee48e5 100644
>>> --- a/sysdeps/unix/sysv/linux/m68k/m68k-helpers.c
>>> +++ b/sysdeps/unix/sysv/linux/m68k/m68k-helpers.c
>>> @@ -17,9 +17,84 @@
>>> <https://www.gnu.org/licenses/>. */
>>>
>>> #include <sysdep.h>
>>> +#include <stdint.h>
>>> +#include <sys/auxv.h>
>>>
>>> +/*
>>> + * vDSO data structure - must match the kernel's m68k_vdso_data struct.
>>> + * The TLS pointer is at offset 0 in the vDSO data page.
>>> + */
>>> +struct m68k_vdso_data
>>> +{
>>> + unsigned long tls_ptr;
>>> + unsigned long reserved[3];
>>> +};
>>> +
>>> +/*
>>> + * Pointer to the vDSO data page.
>>> + * 0 = not yet initialized
>>> + * -1 = vDSO not available (fallback to syscall)
>>> + * other = valid vDSO data page address
>>> + */
>>> +static volatile struct m68k_vdso_data *__m68k_vdso_data;
>>
>> This should be done at process initialization, similar how the other vDSO
>> pointer are done at sysdeps/unix/sysv/linux/dl-vdso-setup.h. Most likely
>> it would require a arch-specific hook where to add the logic.
>>
>> This would allow to optimize the call to avoid the branch to check if the
>> vDSO is provided (with the fallback being the syscall), similar to how iFUNC
>> are internally implemented; and by putting the pointer on the RELRO it would
>> not require to add any hardening (as with PTR_MANGLE).
>
> You're right. I can rework this to:
> 1. Initialize at process startup via dl-vdso-setup.h hooks
> 2. Use an iFUNC-like pattern to avoid the branch on every call
> 3. Store the pointer in RELRO to avoid PTR_MANGLE overhead
>
>>> +
>>> +/*
>>> + * Initialize the vDSO pointer on first use.
>>> + * Returns the pointer to the vDSO data, or NULL if not available.
>>> + */
>>> +static inline struct m68k_vdso_data *
>>> +__m68k_vdso_get (void)
>>> +{
>>> + struct m68k_vdso_data *vdso = (struct m68k_vdso_data *) __m68k_vdso_data;
>>> +
>>> + if (__glibc_unlikely (vdso == NULL))
>>> + {
>>> + /* Not yet initialized - try to get vDSO base from auxv */
>>> + unsigned long sysinfo_ehdr = __getauxval (AT_SYSINFO_EHDR);
>>> + if (sysinfo_ehdr != 0)
>>> + {
>>> + /*
>>> + * The kernel passes the address of the vDSO ELF header (code page).
>>> + * The vDSO memory layout is:
>>> + * base + 0x0000: vvar (data page with TLS pointer, clock state)
>>> + * base + 0x1000: timer page (hardware timer registers)
>>> + * base + 0x2000: vDSO code (ELF, AT_SYSINFO_EHDR points here)
>>> + * So the data page is 2 * PAGE_SIZE before the code page.
>>> + */
>>> + vdso = (struct m68k_vdso_data *) (sysinfo_ehdr - 2 * 4096);
>>
>> This is extremely fragile, meaning that if kernel vvar, timer page, or
>> the vDSO code increase its size it would break. On other architecture
>> this is transparent and vDSO can be extended any required vvar or code
>> pages (and it was done for some architecture which extended code required
>> pages to add new functionalities like the vgetrandom).
>
> This is the same concern Florian raised. I'll rework the kernel side
> to export a __vdso_get_thread_area symbol that glibc can call, rather
> than having glibc calculate offsets into the vDSO memory layout.
>
>>
>> This address needs to be provided by a vDSO symbol, complete transparent
>> to the userland.
>>
>> Also, 4096 is wrong here because m68k/coldfire have 8k page sizes.
>
> I have fixed this in a later revision in my internal tree to use
> __getauxval(AT_PAGESZ) for dynamic page size detection. I'll make sure
> the updated patch is sent once I address the other feedback.
We have GLRO(dl_pagesize) that already has the AUXV value of the pagesize,
you just need to check if its initialization is done prior the vDSO setup
(assuming you intend to initialize at program startup).
But I think if the vDSO exports a __vdso_get_thread_area, there is no need
to take the page size in consideration.
>
> I'll revise both the kernel and glibc patches to follow the standard
> vDSO patterns before resubmitting.
>
>>
>>> + }
>>> + else
>>> + {
>>> + /* vDSO not available - mark as unavailable */
>>> + vdso = (struct m68k_vdso_data *) (uintptr_t) -1;
>>> + }
>>> + __m68k_vdso_data = vdso;
>>> + }
>>> +
>>> + /* Return NULL if vDSO is marked as unavailable */
>>> + if ((uintptr_t) vdso == (uintptr_t) -1)
>>> + return NULL;
>>> +
>>> + return vdso;
>>> +}
>>> +
>>> +/*
>>> + * Get the thread pointer.
>>> + *
>>> + * Fast path: Read directly from the vDSO data page if available.
>>> + * The kernel updates this on every context switch and set_thread_area() call.
>>> + *
>>> + * Slow path: Fall back to the get_thread_area syscall.
>>> + */
>>> void *
>>> __m68k_read_tp (void)
>>> {
>>> - return (void*) INTERNAL_SYSCALL_CALL (get_thread_area);
>>> + struct m68k_vdso_data *vdso = __m68k_vdso_get ();
>>> +
>>> + /* Fast path: use vDSO if available */
>>> + if (__glibc_likely (vdso != NULL))
>>> + return (void *) vdso->tls_ptr;
>>> +
>>> + /* Slow path: syscall fallback */
>>> + return (void *) INTERNAL_SYSCALL_CALL (get_thread_area);
>>> }
>>
>
> Stefan
Reply to: