Re: glibc 2.21 - Machine maintainers, please test your machines.

[Torvald Riegel <triegel at redhat dot com>]

On Sat, 2015-01-24 at 10:14 -0500, Chris Metcalf wrote:
> On 1/23/2015 4:32 PM, Carlos O'Donell wrote:
> > Dear Machine Maintainers,
> >
> > Please start testing your machines against glibc
> > master.
> >
> > Please update the glibc 2.21 release page with your
> > testing results:
> >
> > https://sourceware.org/glibc/wiki/Release/2.21
> >
> > If nobody objects I want to cut the release as soon
> > as we have results for all the machines.
> >
> > Cheers,
> > Carlos.
> 
> tilegx64 is fine (modulo the one bug compiler bug that has been outstanding for multiple glibc releases now).
> 
> tilegx32 has a bunch of new failures, all of which manifest as bus errors:
> 
> FAIL: nptl/tst-sem14
> FAIL: nptl/tst-sem3
> FAIL: nptl/tst-sem6
> FAIL: nptl/tst-signal3
> FAIL: nptl/tst-tls2
> FAIL: nptl/tst-tls3
> 
> I assume these are all from the new semaphore code but have not had an opportunity to look more closely.  One thing that could cause this is if somehow we are trying to do atomic operations on 64-bit values that aren't aligned to an 8-byte boundary.
> 

One thing you could test is set __HAVE_64B_ATOMICS to 0 on tilegx32, and
see whether it makes the semaphore tests pass.  If it avoids the issue,
what is __alignof (sem_t) on tilegx32?

struct new_sem (sysdeps/nptl/internaltypes.h) was changed from having an
"unsigned long int" as biggest field to having "unsigned long long int"
or "uint64_t" fields.  On the x86_64 ABI, this should both result in an
8-byte alignment I believe.

[I'm still looking at how the different archs define sem_t.  But saw
your email, so dumping this email here for now...]

I suppose that's not the case on the tilegx32 ABI?  If it is, we can't
use the 64b algorithm there if we don't want to change the glibc ABI,
AFAICT.

A simple "fix" is to not make 64B atomics available.  However, the issue
is the alignment change; IOW, whether "unsigned long" is 8B-aligned or
not.  Adding such a check to struct new_sem and in the semaphore code
would be better, I suppose.  It would be even better if we would add
another flag to each atomic.h stating the minimum alignment requirements
for 64b atomic ops.

HJ, do we have the same issue on x32?  IOW, same alignment issue, and
64b atomics need to be targetting naturally aligned data?

I'd appreciate feedback on other archs wrt the alignment question and
alignment requirements for 64b atomics.

We have several options here, and while I agree there's an issue and I
hadn't considered the alignment of struct new_sem sufficiently, just
using the 32b semaphore variant is always an easy option.


There might be another issue in the 32b variant of struct new_sem, which
changed like this:
-  unsigned long int nwaiters;
+  int pad;
+  unsigned int nwaiters;

I'll investigate this further.  The size increase might be an issue.

Do we know whether all our targets with just 32b atomic operations are
ILP32?  If yes, then removing the addition of "int pad;" should fix
this.  If not, then would adding a union like this be most useful?
  union { unsigned long int dummy; 


Sorry about the fallout!  I'll try to fix that up quickly.