This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] [BZ 18034] [AArch64] Lazy TLSDESC relocation data race fix


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!


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]