[PATCH] 8170639: [Linux] jsig is limited to a maximum of 64 signals
Ao Qi
aoqi at loongson.cn
Tue Dec 18 01:13:25 UTC 2018
Hi,
On Tue, Dec 18, 2018 at 8:33 AM David Holmes <david.holmes at oracle.com> wrote:
>
> On 18/12/2018 9:57 am, Ao Qi wrote:
> > Hi,
> >
> > On Tue, Dec 18, 2018 at 4:32 AM David Holmes <david.holmes at oracle.com> wrote:
> >>
> >> On 17/12/2018 9:48 pm, Ao Qi wrote:
> >>> Hi David Holmes,
> >>>
> >>> Thanks for your reply. It makes sense. Is it possible to create
> >>> another bug ID to fix the problem?
> >>
> >> I'd prefer to see this fixed properly than point fixed on linux for an
> >> unsupported platform.
> >
> > I thought the problem is "[Linux] jsig is limited to a maximum of 64
> > signals" and the patch was intended to fixed this problem. It just
> > didn't use "A better solution", but I think it is better than the
> > current situation.
>
> Better for you but I would prefer to see an overall better solution. It
> really shouldn't be very difficult. Compare the AIX version with your
> improved Linux version, reconcile any differences, and make it a shared
> posix version, then confirm it works on OS X and Solaris.
>
> > Is zero/mips (or all platforms having more than 64 signals) a
> > unsupported platform?
>
> Yes. We make allowances for Zero as it can support a vast range of
> platforms for which no native OpenJDK port exists (in the case of MIPS
> that port seems inactive), but in general we don't go out of our way to
> make changes in the main code that only benefit such platforms.
>
> I'm subtly trying to coerce you into making the change I'd like to see
> here. ;-) I can assist with build/test on other POSIX platforms.
>
Thanks for your explanation and suggestion. I thought the linux way
may be a possibility in order that the last patch making zero/mips
built can be pushed, so companies which build zero/mips, like Debian
and Loongson, do not need their own patch anymore (which was used for
years).
I have not worked on the platforms other than linux and do not have
these platforms for test. I will try it according to your suggestion:)
> Cheers,
> David
>
> > Cheers,
> > Ao Qi
> >>
> >> Thanks,
> >> David
> >>
> >>> Cheers,
> >>> Ao Qi
> >>>
> >>> On Mon, Dec 17, 2018 at 7:30 PM David Holmes <david.holmes at oracle.com> wrote:
> >>>>
> >>>> Thanks for reminding me about the rest of the discussion. The point of
> >>>> the RFE I filed was to examine if we could get a consolidated POSIX
> >>>> version using sigset_t. I'd rather see that than a point fix for Linux.
> >>>>
> >>>> Thanks,
> >>>> David
> >>>>
> >>>> On 17/12/2018 6:50 pm, Ao Qi wrote:
> >>>>> Hi David Holmes,
> >>>>>
> >>>>> I am not sure what is the specific issue of "NSIG not being defined".
> >>>>> Do you mean the issue mentioned in "but I found that NSIG was missing
> >>>>> from signal.h on some architectures, mips being among them"[1]? If
> >>>>> yes, James Cowgill answered this in [2]. I check it on MIPS, NSIG is
> >>>>> defined in signal.h and it is 128.
> >>>>>
> >>>>> Cheers,
> >>>>> Ao Qi
> >>>>>
> >>>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-November/025401.html
> >>>>> [2] http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-December/025416.html
> >>>>>
> >>>>> On Mon, Dec 17, 2018 at 8:02 AM David Holmes <david.holmes at oracle.com> wrote:
> >>>>>>
> >>>>>> Hi,
> >>>>>>
> >>>>>> Was the issue of NSIG not being defined resolved?
> >>>>>>
> >>>>>> Thanks,
> >>>>>> David
> >>>>>>
> >>>>>> On 14/12/2018 10:23 pm, Ao Qi wrote:
> >>>>>>> Hi all,
> >>>>>>>
> >>>>>>> Zero does not build on Linux MIPS (actually all Linux of NSIG > 64)
> >>>>>>> since OpenJDK 9, because NISG is 128 on MIPS and can not be encoded in
> >>>>>>> uint64_t sigs. James Cowgill from Debian tried to push a patch fixing
> >>>>>>> this[1] but it seems he failed. I would like to try again to request
> >>>>>>> fixing this in http://hg.openjdk.java.net/jdk/jdk. This was fixed in
> >>>>>>> AIX implementation.
> >>>>>>>
> >>>>>>> After applying this patch, zero can be built on MIPS (Debian testing).
> >>>>>>> I've run the jtreg tests [2] on x86 Linux.
> >>>>>>>
> >>>>>>> [1] http://mail.openjdk.java.net/pipermail/hotspot-dev/2016-November/025399.html
> >>>>>>> [2] https://download.java.net/openjdk/testresults/12/docs/howtoruntests.html
> >>>>>>>
> >>>>>>> Cheers,
> >>>>>>> Ao Qi
> >>>>>>>
> >>>>>>> $ hg export -r 52869
> >>>>>>> # HG changeset patch
> >>>>>>> # User aoqi
> >>>>>>> # Date 1544089853 0
> >>>>>>> # Thu Dec 06 09:50:53 2018 +0000
> >>>>>>> # Node ID 3eea22b79dc3a4bf26616bde1acb587e3f56e6fe
> >>>>>>> # Parent b4982a22926b4ddf1a7b1f770e4d42ce8c1dd575
> >>>>>>> 8170639: [Linux] jsig is limited to a maximum of 64 signals
> >>>>>>>
> >>>>>>> diff -r b4982a22926b -r 3eea22b79dc3 src/hotspot/os/linux/os_linux.cpp
> >>>>>>> --- a/src/hotspot/os/linux/os_linux.cpp Thu Dec 06 11:54:39 2018 +0530
> >>>>>>> +++ b/src/hotspot/os/linux/os_linux.cpp Thu Dec 06 09:50:53 2018 +0000
> >>>>>>> @@ -4434,10 +4434,7 @@
> >>>>>>>
> >>>>>>> // For signal-chaining
> >>>>>>> struct sigaction sigact[NSIG];
> >>>>>>> -uint64_t sigs = 0;
> >>>>>>> -#if (64 < NSIG-1)
> >>>>>>> -#error "Not all signals can be encoded in sigs. Adapt its type!"
> >>>>>>> -#endif
> >>>>>>> +sigset_t sigs;
> >>>>>>> bool os::Linux::libjsig_is_loaded = false;
> >>>>>>> typedef struct sigaction *(*get_signal_t)(int);
> >>>>>>> get_signal_t os::Linux::get_signal_action = NULL;
> >>>>>>> @@ -4516,7 +4513,7 @@
> >>>>>>> }
> >>>>>>>
> >>>>>>> struct sigaction* os::Linux::get_preinstalled_handler(int sig) {
> >>>>>>> - if ((((uint64_t)1 << (sig-1)) & sigs) != 0) {
> >>>>>>> + if (sigismember(&sigs, sig)) {
> >>>>>>> return &sigact[sig];
> >>>>>>> }
> >>>>>>> return NULL;
> >>>>>>> @@ -4525,7 +4522,7 @@
> >>>>>>> void os::Linux::save_preinstalled_handler(int sig, struct sigaction& oldAct) {
> >>>>>>> assert(sig > 0 && sig < NSIG, "vm signal out of expected range");
> >>>>>>> sigact[sig] = oldAct;
> >>>>>>> - sigs |= (uint64_t)1 << (sig-1);
> >>>>>>> + sigaddset(&sigs, sig);
> >>>>>>> }
> >>>>>>>
> >>>>>>> // for diagnostic
> >>>>>>> @@ -4616,6 +4613,7 @@
> >>>>>>> (*begin_signal_setting)();
> >>>>>>> }
> >>>>>>>
> >>>>>>> + ::sigemptyset(&sigs);
> >>>>>>> set_signal_handler(SIGSEGV, true);
> >>>>>>> set_signal_handler(SIGPIPE, true);
> >>>>>>> set_signal_handler(SIGBUS, true);
> >>>>>>>
More information about the hotspot-runtime-dev
mailing list