This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] [BZ 18034] [AArch64] Lazy TLSDESC relocation data race fix
- From: Torvald Riegel <triegel at redhat dot com>
- To: Szabolcs Nagy <szabolcs dot nagy at arm dot com>
- Cc: "libc-alpha at sourceware dot org" <libc-alpha at sourceware dot org>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Ramana Radhakrishnan <Ramana dot Radhakrishnan at arm dot com>
- Date: Wed, 03 Jun 2015 12:40:13 +0200
- Subject: Re: [PATCH] [BZ 18034] [AArch64] Lazy TLSDESC relocation data race fix
- Authentication-results: sourceware.org; auth=none
- References: <553793A3 dot 7030206 at arm dot com> <1429718899 dot 6557 dot 17 dot camel at triegel dot csb> <553E5381 dot 504 at arm dot com> <1432672677 dot 26239 dot 41 dot camel at triegel dot csb> <5565AA2A dot 7010509 at arm dot com> <1432731762 dot 30849 dot 53 dot camel at triegel dot csb> <556C3301 dot 1070007 at arm dot com>
On Mon, 2015-06-01 at 11:25 +0100, Szabolcs Nagy wrote:
> i added a comment to the _dl_tlsdesc_resolve_early_return_p
> call in aarch64 tlsdesc.c about the retry loop.
That's good, but it would have been better if you could have briefly
pointed out that this relates to mo_relaxed loads in
_dl_tlsdesc_resolve_early_return_p. And/or added a comment there saying
that the mo_relaxed loads are fine because of this retry loop in the
caller(s).
> diff --git a/sysdeps/aarch64/tlsdesc.c b/sysdeps/aarch64/tlsdesc.c
> index 4821f8c..d2f1cd5 100644
> --- a/sysdeps/aarch64/tlsdesc.c
> +++ b/sysdeps/aarch64/tlsdesc.c
> @@ -39,11 +40,12 @@
>
> void
> attribute_hidden
> -_dl_tlsdesc_resolve_rela_fixup (struct tlsdesc volatile *td,
> - struct link_map *l)
> +_dl_tlsdesc_resolve_rela_fixup (struct tlsdesc *td, struct link_map
> *l)
> {
> - const ElfW(Rela) *reloc = td->arg;
> + const ElfW(Rela) *reloc = atomic_load_relaxed (&td->arg);
Good change. Can you add a brief comment saying why the mo_relaxed load
is sufficient? IIRC, this is because of the acquire loads done by the
caller.
OK with those changes.
Thanks!