Hotspot for BSD / opinion request

[David Holmes]

Filed: https://bugs.openjdk.java.net/browse/JDK-8174231

Thanks,
David H.

On 8/02/2017 12:32 PM, David Holmes wrote:
> Hi David,
>
> Sorry for the long lag on this. I've been looking into the mutex
> attribute defaults and it is unfortunate that POSIX allowed
> PTHREAD_MUTEX_DEFAULT to be an implementation specific alias for one of
> the other three mutex types: PTHREAD_MUTEX_NORMAL,
> PTHREAD_MUTEX_ERRORCHECK, PTHREAD_MUTEX_RECURSIVE. But even more
> unfortunate that FreeBSD decided to map it to ERRORCHECK instead of
> NORMAL as every other platform seems to do! (I note NetBSD defines a
> DEFAULT that is compatible with NORMAL, not ERRORCHECK.)
>
> Fixing this for FreeBSD when it is not an officially supported platform
> is a little messy because we don't want to penalize all the other
> platforms with extra initialization code. Right now this could be
> confined to os_bsd.* but this code should be refactored into os_posix.*
> to be more widely shared.
>
> That aside we do not need to keep an attr object per mutex as they will
> all share the same attributes. So we would have a single static instance
> and initialize it once during VM initialization, then use it when
> creating mutexes. Even then I would structure this so that the use of
> the attr object is hidden behind a macro so that it is a no-op on
> platforms that use NORMAL as the default.
>
> I will file a RFE for sharing POSIX-based PlatformEvent/Parker code and
> include the mutex type issue as part of that.
>
> Thanks,
> David H.
> -------
>
>
> On 10/01/2017 3:37 AM, David CARLIER wrote:
>> Hi sure thing
>>
>> diff --git a/src/os/bsd/vm/os_bsd.hpp b/src/os/bsd/vm/os_bsd.hpp
>> --- a/src/os/bsd/vm/os_bsd.hpp
>> +++ b/src/os/bsd/vm/os_bsd.hpp
>> @@ -175,6 +175,7 @@
>>    volatile int _Event;
>>    volatile int _nParked;
>>    pthread_mutex_t _mutex[1];
>> +  pthread_mutexattr_t _attr[1];
>>    pthread_cond_t  _cond[1];
>>    double PostPad[2];
>>    Thread * _Assoc;
>> @@ -187,7 +188,11 @@
>>      int status;
>>      status = pthread_cond_init(_cond, NULL);
>>      assert_status(status == 0, status, "cond_init");
>> -    status = pthread_mutex_init(_mutex, NULL);
>> +    status = pthread_mutexattr_init(_attr);
>> +    assert_status(status == 0, status, "attr_init");
>> +    status = pthread_mutexattr_settype(_attr, PTHREAD_MUTEX_NORMAL);
>> +    assert_status(status == 0, status, "attr_settype");
>> +    status = pthread_mutex_init(_mutex, _attr);
>>      assert_status(status == 0, status, "mutex_init");
>>      _Event   = 0;
>>      _nParked = 0;
>> @@ -206,6 +211,7 @@
>>  class PlatformParker : public CHeapObj<mtInternal> {
>>   protected:
>>    pthread_mutex_t _mutex[1];
>> +  pthread_mutexattr_t _attr[1];
>>    pthread_cond_t  _cond[1];
>>
>>   public:       // TODO-FIXME: make dtor private
>> @@ -216,7 +222,11 @@
>>      int status;
>>      status = pthread_cond_init(_cond, NULL);
>>      assert_status(status == 0, status, "cond_init");
>> -    status = pthread_mutex_init(_mutex, NULL);
>> +    status = pthread_mutexattr_init(_attr);
>> +    assert_status(status == 0, status, "attr_init");
>> +    status = pthread_mutexattr_settype(_attr, PTHREAD_MUTEX_NORMAL);
>> +    assert_status(status == 0, status, "attr_settype");
>> +    status = pthread_mutex_init(_mutex, _attr);
>>      assert_status(status == 0, status, "mutex_init");
>>    }
>>  };
>>
>>
>>
>> Kind regards.
>>
>> Thanks.
>>
>> On 9 January 2017 at 16:48, Daniel D. Daugherty
>> <daniel.daugherty at oracle.com> wrote:
>>> David C.,
>>>
>>> If you sent the patch as an attachment, then you should be advised that
>>> the OpenJDK email servers strip attachments. If the patch is relatively
>>> small, then you can send it in-line...
>>>
>>> Dan
>>>
>>>
>>>
>>> On 1/6/17 3:23 PM, David CARLIER wrote:
>>>>
>>>> Thanks, here a patch proposal. Kindest regards.
>>>>
>>>> On 24 December 2016 at 02:16, David Holmes <david.holmes at oracle.com>
>>>> wrote:
>>>>>
>>>>> If there is a platform, we support, where the default settings are
>>>>> inappropriate then setting them explicitly would be the right thing to
>>>>> do.
>>>>>
>>>>> David
>>>>>
>>>>>
>>>>> On 24/12/2016 10:23 AM, Vladimir Kozlov wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Forwarding to mailing lists since I can't answer.
>>>>>> May be someone can answer your question on these lists.
>>>>>>
>>>>>> Regards,
>>>>>> Vladimir
>>>>>>
>>>>>> On 12/23/16 2:17 AM, David CARLIER wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> this is my first mail to the maiing list, I have difficulties to
>>>>>>> push
>>>>>>> messages to any mailing lists, but as BSD user/dev I was
>>>>>>> looking at code's part like this one
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> http://hg.openjdk.java.net/jdk9/dev/hotspot/file/70c6fae64754/src/os/bsd/vm/os_bsd.hpp
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I was wondering if it would be better to explicitally set the attr
>>>>>>> type to PTHREAD_MUTEX_NORMAL to insure consistency with AIX and
>>>>>>> Linux
>>>>>>> (on FreeBSD it is PTHREAD_MUTEX_ERRORCHECK by default).
>>>>>>>
>>>>>>> What do you think ?
>>>>>>>
>>>>>>> Thanks in advance.
>>>>>>>
>>>>>>> Kindest regards.
>>>>>>>
>>>