From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: David Mozes <david.mozes@silk.us>
Cc: Christian Hoff <christian_hoff@gmx.net>,
Florian Weimer <fweimer@redhat.com>,
"libc-help@sourceware.org" <libc-help@sourceware.org>,
Carlos O'Donell <carlos@redhat.com>
Subject: Re: rwlock different behavior on glibc 2.28 then on previous versions
Date: Mon, 21 Mar 2022 17:19:49 -0300 [thread overview]
Message-ID: <0599084c-49bc-23a3-da7e-ccb197d4c88c@linaro.org> (raw)
In-Reply-To: <6D2D3001-2308-4336-829F-8369DF7D2514@silk.us>
On 21/03/2022 16:46, David Mozes wrote:
>
> Thanks Adhemerval,
> For the suggestion!
>
> I suggest to remove the check just on the unlock function.I tested it with very high load and with application with 3k active threads.
> Can you on a case or bug(dedlock or something) that removing thIs check will expose?
I am no sure, so I would suggest you to maybe start a discussion on libc-alpha.
However __pthread_rwlock_wrunlock and __pthread_rwlock_rdunlock have different
semantic allowing unlocking write locks on different threads will require a
lot of analysis on why it would trigger any more regression (and sorry, but
stating it works on our environment is not suffice).
> Thx
> David
>> On Mar 21, 2022, at 7:02 PM, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>>
>> As Florian has put, this is essentially a misuse of the API has always been
>> undefined. The rwlock implementation uses __cur_write to both make a fast
>> deadlock check and to know whether to handle the lock as a writer or reader
>> one. The former is a implementation detail that we might change (although
>> it is useful in some case), but to handle the former will most likely to
>> refactor and change the rdlock algorithm (just removing the check as you are
>> proposing is not suffice).
>>
>> Since it is not a bug in our implementation, I think you will need to discuss
>> it further on the libc-alpha to see if other are willing to actually make
>> this change. I foresee that the algorithm change won't be simple to keep
>> interface requirements and allow the specific case you are asking. In fact
>> I think it might not be actually possible without change the rwlock semantic
>> (not sure though).
>>
>> Carlos has fixed some issue on the rwlock, he might have some comments.
>>
>> On 21/03/2022 11:01, David Mozes wrote:
>>> Hello Christain,
>>> Yes, Thread Sanitizer caught it.
>>> However, we can't run it all the time on the production code.
>>> We need to make a small change of the unlock code to have async programing and backward compatibility for older code.
>>>
>>> Thx
>>> David
>>>
>>> -----Original Message-----
>>> From: Christian Hoff <christian_hoff@gmx.net>
>>> Sent: Monday, March 21, 2022 8:20 AM
>>> To: David Mozes <david.mozes@silk.us>; Florian Weimer <fweimer@redhat.com>
>>> Cc: libc-help@sourceware.org
>>> Subject: Re: rwlock different behavior on glibc 2.28 then on previous versions
>>>
>>> Hallo David,
>>>
>>> did you try to run your program with Helgrind already? Helgrind was
>>> designed to catch problems like this. Maybe Thread Sanitizer would also
>>> detect the issue.
>>>
>>>
>>> Best regards,
>>>
>>> Christian
>>>
>>> On 3/20/22 5:44 PM, David Mozes wrote:
>>>> Hi,
>>>> Where are we with this?
>>>> Also, the current implementation of the __pthread_rwlock_unlock is problematic.
>>>> On the one hand, it checks the API agreement. However, on the case of violation, the user don't have any induction
>>>> And function return OK
>>>> See example code below:
>>>>
>>>>
>>>> struct threadInfo{
>>>> pthread_t* tid;
>>>> pthread_rwlock_t *rwlock;
>>>> };
>>>>
>>>> void *lockerThreadFunction(void *vargp)
>>>> {
>>>> km_status stat = KM_OK;
>>>> struct threadInfo* Tinfo = (struct threadInfo*)vargp;
>>>>
>>>> do {
>>>> stat = pthread_rwlock_trywrlock(Tinfo->rwlock);
>>>> if (KM_OK == stat) {
>>>> printf("[INFO] - FIRST thread 0x%x locked %p \n", *(Tinfo->tid), Tinfo->rwlock);
>>>> sleep(10);
>>>> stat = pthread_rwlock_unlock(Tinfo->rwlock);
>>>> if (KM_OK == stat) {
>>>> printf("[INFO] FIRST thread 0x%x released %p \n", *(Tinfo->tid), Tinfo->rwlock);
>>>> sleep(1);
>>>> }
>>>> }
>>>> }while(KM_OK != stat);
>>>> return NULL;
>>>> }
>>>>
>>>>
>>>> void *secondThreadFunction(void *vargp)
>>>> {
>>>> km_status stat = KM_OK;
>>>> struct threadInfo* Tinfo = (struct threadInfo*)vargp;
>>>>
>>>> sleep(5);
>>>> do {
>>>> stat = pthread_rwlock_unlock(Tinfo->rwlock);
>>>> if (KM_OK == stat) {
>>>> printf("[INFO] SECOND thread 0x%x released %p \n", *(Tinfo->tid), Tinfo->rwlock);
>>>> sleep(1);
>>>> } else
>>>> printf("[INFO] SECOND thread 0x%x try_to_released %p erro status is: %d \n", *(Tinfo->tid), Tinfo->rwlock,stat);
>>>>
>>>> }while(KM_OK != stat);
>>>> return NULL;
>>>> }
>>>>
>>>> void *thirdThreadFunction(void *vargp)
>>>> {
>>>> km_status stat = KM_OK;
>>>> struct threadInfo* Tinfo = (struct threadInfo*)vargp;
>>>>
>>>> sleep(2);
>>>> do {
>>>> stat = pthread_rwlock_trywrlock(Tinfo->rwlock);
>>>> if (KM_OK == stat) {
>>>> printf("[INFO] THIRD thread 0X%x locked %x\n", *(Tinfo->tid), Tinfo->rwlock);
>>>> sleep(5);
>>>> }
>>>> else {
>>>> printf("[ERROR]: THIRD thread 0x%x failed Locking %p status is %d\n", *(Tinfo->tid), Tinfo->rwlock,stat);
>>>> sleep(1);
>>>> }
>>>> }while(KM_OK != stat);
>>>> return NULL;
>>>> }
>>>>
>>>> void main()
>>>> {
>>>> km_status stat = KM_OK;
>>>> /* km_rwlock lock; */
>>>> pthread_rwlock_t rwlock;
>>>> pthread_t lockerTid = 0, releaseTid = 0, thirdTid=0;
>>>> struct threadInfo firstTinfo, secondTinfo, thirdTinfo;
>>>>
>>>> /* stat = km_rwlock_init(&rwlock, KM_TRUE); */
>>>> stat = pthread_rwlock_init (&rwlock, NULL);
>>>>
>>>> if (KM_OK == stat) {
>>>> printf("read\\writer lock initialized\n");
>>>> printf("Before Thread\n");
>>>>
>>>> firstTinfo.tid = &lockerTid;
>>>> firstTinfo.rwlock = &rwlock;
>>>> secondTinfo.tid = &releaseTid;
>>>> secondTinfo.rwlock = &rwlock;
>>>> thirdTinfo.tid = &thirdTid;
>>>> thirdTinfo.rwlock = &rwlock;
>>>>
>>>>
>>>> pthread_create(&lockerTid, NULL, lockerThreadFunction, &firstTinfo);
>>>> pthread_create(&releaseTid, NULL, secondThreadFunction, &secondTinfo);
>>>> pthread_create(&thirdTid, NULL, thirdThreadFunction, &thirdTinfo);
>>>> printf("After Thread %x\n", lockerTid);
>>>>
>>>> pthread_join(lockerTid, NULL);
>>>> pthread_join(releaseTid, NULL);
>>>> pthread_join(thirdTid, NULL);
>>>>
>>>>
>>>>
>>>> the output we saw is this:
>>>>
>>>>
>>>> the ouput we saw is this:
>>>>
>>>> [root@Rocky85Mozes thread_lock]# ./rw_test
>>>> read\writer lock initialized
>>>> Before Thread
>>>> [INFO] - FIRST thread 0x461f700 locked 0x7ffcda053cc0
>>>> After Thread 461f700
>>>> [ERROR]: THIRD thread 0x361d700 failed Locking 0x7ffcda053cc0 status is 16 -> first thread take the rwlock
>>>> [ERROR]: THIRD thread 0x361d700 failed Locking 0x7ffcda053cc0 status is 16
>>>> [ERROR]: THIRD thread 0x361d700 failed Locking 0x7ffcda053cc0 status is 16
>>>> [INFO] SECOND thread 0x3e1e700 released 0x7ffcda053cc0 -> Second thread unlock the rwlock -- > and get OK!!
>>>> [ERROR]: THIRD thread 0x361d700 failed Locking 0x7ffcda053cc0 status is 16
>>>> [ERROR]: THIRD thread 0x361d700 failed Locking 0x7ffcda053cc0 status is 16
>>>> [ERROR]: THIRD thread 0x361d700 failed Locking 0x7ffcda053cc0 status is 16
>>>> [ERROR]: THIRD thread 0x361d700 failed Locking 0x7ffcda053cc0 status is 16
>>>> [ERROR]: THIRD thread 0x361d700 failed Locking 0x7ffcda053cc0 status is 16
>>>> [INFO] FIRST thread 0x461f700 released 0x7ffcda053cc0
>>>> [ERROR]: THIRD thread 0x361d700 failed Locking 0x7ffcda053cc0 status is 16
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: David Mozes
>>>> Sent: Sunday, March 13, 2022 12:23 PM
>>>> To: Florian Weimer <fweimer@redhat.com>
>>>> Cc: libc-help@sourceware.org
>>>> Subject: RE: rwlock different behavior on glibc 2.28 then on previous versions
>>>>
>>>> Thx Florin,
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: Florian Weimer <fweimer@redhat.com>
>>>> Sent: Friday, March 11, 2022 3:59 PM
>>>> To: David Mozes <david.mozes@silk.us>
>>>> Cc: libc-help@sourceware.org
>>>> Subject: Re: rwlock different behavior on glibc 2.28 then on previous versions
>>>>
>>>> * David Mozes:
>>>>
>>>>> On the previous version of Glibc (before the commit) We could release
>>>>> write lock with different thread as the one that owned the lock (If we
>>>>> like) .
>>>> Sorry, this misuse of the API has always been undefined according to
>>>> POSIX (“Results are undefined if the read-write lock rwlock is not held
>>>> by the calling thread.”).
>>>>
>>>>>> Correct, but practically we can do it on the user's responsibility.
>>>>>> Do you know if this "if" solved some synchronization issue or just enforced the undefined API?
>>>>> This is help very much for async program that has a many threads that
>>>>> communicate with some target. And we like to define a call back
>>>>> function instead of blocking on aio thread that send the data in order
>>>>> to release the rwlock.this improves the performance dramatically .
>>>> Could you use a different synchronization object (perhaps a condition
>>>> variable)?
>>>>
>>>>>> If we use a different synchronization object like semaphores, condition variable (by the way they use mutex under the same API agreement), we will need to write self rwlock , which has less performance than other undefined states.
>>>>>> If we like to keep the API agreement, why not define the different async programming unlock functions?
>>>> Thanks
>>>> Florian
>>>>
>>>> Thx
>>>> David
next prev parent reply other threads:[~2022-03-21 20:19 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-07 20:13 David Mozes
2022-03-11 13:58 ` Florian Weimer
2022-03-13 10:23 ` David Mozes
2022-03-20 16:44 ` David Mozes
2022-03-21 6:19 ` Christian Hoff
2022-03-21 14:01 ` David Mozes
2022-03-21 17:01 ` Adhemerval Zanella
2022-03-21 19:46 ` David Mozes
2022-03-21 20:19 ` Adhemerval Zanella [this message]
2022-03-24 15:41 ` David Mozes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0599084c-49bc-23a3-da7e-ccb197d4c88c@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=carlos@redhat.com \
--cc=christian_hoff@gmx.net \
--cc=david.mozes@silk.us \
--cc=fweimer@redhat.com \
--cc=libc-help@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).