Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.bugs.dist > #1193906 > unrolled thread

Bug#1068873: openjdk-21: more m68k patches

Back to article view | Back to linux.debian.bugs.dist

Contents

  Bug#1068873: openjdk-21: more m68k patches Thorsten Glaser <tg@mirbsd.de> - 2024-04-12 18:40 +0200
    Bug#1068873: openjdk-21: more m68k patches Thorsten Glaser <tg@mirbsd.de> - 2024-04-13 00:50 +0200
      Bug#1068873: openjdk-21: more m68k patches Thorsten Glaser <tg@mirbsd.de> - 2024-04-13 02:00 +0200
        Bug#1068873: openjdk-21: more m68k patches Thorsten Glaser <tg@mirbsd.de> - 2024-04-13 02:00 +0200
          Bug#1068873: openjdk-21: more m68k patches Thorsten Glaser <tg@mirbsd.de> - 2024-04-15 12:50 +0200

#1193906 — Bug#1068873: openjdk-21: more m68k patches

Source: openjdk-21
X-Debbugs-Cc: tg@mirbsd.de, debian-68k@lists.debian.org

Please add the following patch e.g. to debian/patches/m68k-support.diff
for more making implicit alignment assumptions (here by the futex
syscall) explicit:

--- src/hotspot/os/linux/waitBarrier_linux.hpp~	2024-04-12 18:24:38.584686322 +0200
+++ src/hotspot/os/linux/waitBarrier_linux.hpp	2024-04-12 18:24:46.768716977 +0200
@@ -29,7 +29,7 @@
 #include "utilities/globalDefinitions.hpp"
 
 class LinuxWaitBarrier : public CHeapObj<mtInternal> {
-  volatile int _futex_barrier;
+  volatile int _futex_barrier __attribute__((__aligned__(4)));
 
   NONCOPYABLE(LinuxWaitBarrier);
 

Thanks!

(This is what I found trying to build openjdk-20, but it’ll be
needed in 21 as well. Even getting to this point took 13½ days
already…)

[toc] | [next] | [standalone]

#1193945

Dixi quod…

>(This is what I found trying to build openjdk-20, but it’ll be
>needed in 21 as well. Even getting to this point took 13½ days
>already…)

And turns out that this isn’t the cause.

In 17, we’ve got src/hotspot/share/memory/allocation.hpp to
align all CHeapObj, StackObj, MetaspaceObj, etc. classes; this
is gone in 21. So this needs to be brought back instead.

[toc] | [prev] | [next] | [standalone]

#1193953

Dixi quod…

>>(This is what I found trying to build openjdk-20, but it’ll be
>>needed in 21 as well. Even getting to this point took 13½ days
>>already…)
>
>And turns out that this isn’t the cause.
>
>In 17, we’ve got src/hotspot/share/memory/allocation.hpp to
>align all CHeapObj, StackObj, MetaspaceObj, etc. classes; this
>is gone in 21. So this needs to be brought back instead.

Hmmhmm. Since I’m having to build/debug 20 first…

… in 20, StackObj has its alignment bumped manually,
but CHeapObj doesn’t. MetaspaceObj does, ResourceObj
and ArenaObj don’t, AnyObj does.

So I’m guessing we will want to fix up the allocators instead?
(Though raising the alignment for cases where people allocate
them on the stack may still be useful…)

ArenaObj… is not allocated‽

resource_allocate_bytes uses Thread::current()->resource_area()->allocate_bytes
which uses Amalloc which seems to align well.

AllocateHeap uses os::malloc which uses ::malloc (C function?)
in NMT and normal cases. Huh. MallocHeader is 16 bytes, also okay.
The glibc texinfo docs say…
| The address of a block returned by ‘malloc’ or ‘realloc’ in GNU systems
| is always a multiple of eight (or sixteen on 64-bit systems).  If you
… so that should *also* be okay?! Unless that’s not true, anyway…

#define SIZE_SZ (sizeof (INTERNAL_SIZE_T))
#define MALLOC_ALIGNMENT (2 * SIZE_SZ < __alignof__ (long double) \
                          ? __alignof__ (long double) : 2 * SIZE_SZ)

… it should.

So where does the unaligned _futex_barrier member in the
class LinuxWaitBarrier : public CHeapObj<mtInternal> come from?

AFAICS, the caller is:

WaitBarrier* SafepointSynchronize::_wait_barrier;
  _wait_barrier = new WaitBarrier(vmthread);

With:

typedef LinuxWaitBarrier WaitBarrierDefault;
template <typename WaitBarrierImpl>
class WaitBarrierType : public CHeapObj<mtInternal> {
  WaitBarrierImpl _impl;
…
}
typedef WaitBarrierType<WaitBarrierDefault> WaitBarrier;

So the “new WaitBarrier” should call CHeapObj::operator new(size_t size)
TTBOMK (IANAC++Programmer) which calls CHeapObjBase::operator new(size, mtInternal)
= AllocateHeap(size, mtInternal)…

Hmmm. But, oops, I see something more:

src/hotspot/share/services/mallocTracker.hpp:  static const size_t overhead_per_malloc = sizeof(MallocHeader) + sizeof(uint16_t);

That would dealign things… but MallocTracker::record_malloc
only adds sizeof(MallocHeader) and has an assert (unsure if
NDEBUG though) that checks alignment…

I am lost. I can *see* an under-aligned futex barrier in strace.

19270 futex(0xcf80078a, FUTEX_WAKE_PRIVATE, 2147483647) = -1 EINVAL (Invalid argument)

I cannot see how, though.

FWIW, /tmp/buildd/openjdk-20-20.0.2+9/build/jdk/bin/java \
-XX:NativeMemoryTracking=summary -version also crashes, same
with an explicit -XX:NativeMemoryTracking=off :/

I’ll recompile with re-enabled alignment on the missing base
classes like we have in 17. But if someone has ideas ’til then…

Mraw,
//mirabilos
-- 
<igli> exceptions: a truly awful implementation of quite a nice idea.
<igli> just about the worst way you could do something like that, afaic.
<igli> it's like anti-design.  <mirabilos> that too… may I quote you on that?
<igli> sure, tho i doubt anyone will listen ;)

[toc] | [prev] | [next] | [standalone]

#1193955

Dixi quod…
>I’ll recompile with re-enabled alignment on the missing base

Seems to be only one.

--- src/hotspot/share/memory/allocation.hpp~	2024-04-12 23:52:54.000000000 +0000
+++ src/hotspot/share/memory/allocation.hpp	2024-04-12 23:52:56.000000000 +0000
@@ -276,7 +276,7 @@ class CHeapObj {
   void operator delete [] (void* p) {
     CHeapObjBase::operator delete[](p);
   }
-};
+} __attribute__ ((aligned (4)));
 
 // Base class for objects allocated on the stack only.
 // Calling new or delete will result in fatal error.

>classes like we have in 17. But if someone has ideas ’til then…

gn8,
//mirabilos
-- 
This space for rent.

[toc] | [prev] | [next] | [standalone]

#1194169

Dixi quod…

>>I’ll recompile with re-enabled alignment on the missing base
>
>Seems to be only one.
>
>--- src/hotspot/share/memory/allocation.hpp~	2024-04-12 23:52:54.000000000 +0000
>+++ src/hotspot/share/memory/allocation.hpp	2024-04-12 23:52:56.000000000 +0000
>@@ -276,7 +276,7 @@ class CHeapObj {
>   void operator delete [] (void* p) {
>     CHeapObjBase::operator delete[](p);
>   }
>-};
>+} __attribute__ ((aligned (4)));
> 
> // Base class for objects allocated on the stack only.
> // Calling new or delete will result in fatal error.
>
>>classes like we have in 17. But if someone has ideas ’til then…
>
Yes, with this, the build gets much further, so I’d be glad if you
could include this in the next -21 upload (and -20 if you do any
more of these, but probably not, and not necessary on my account).

Thanks,
//mirabilos
-- 
Solange man keine schmutzigen Tricks macht, und ich meine *wirklich*
schmutzige Tricks, wie bei einer doppelt verketteten Liste beide
Pointer XORen und in nur einem Word speichern, funktioniert Boehm ganz
hervorragend.		-- Andreas Bogk über boehm-gc in d.a.s.r

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.bugs.dist

csiph-web