From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22c.google.com (mail-oi1-x22c.google.com [IPv6:2607:f8b0:4864:20::22c]) by sourceware.org (Postfix) with ESMTPS id A61D2385E83F for ; Mon, 21 Mar 2022 17:02:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A61D2385E83F Received: by mail-oi1-x22c.google.com with SMTP id v75so16878367oie.1 for ; Mon, 21 Mar 2022 10:02:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=vHL4toixy1/NKsosD3ctsx7GdNrr/Ni25P/4ZJT6ZsE=; b=f9oYOZb6CRC0rW6sfsewZgYUmo2j5lx+06FE3DEsXAbtXuBaaeRw2fyNQwXBhI/kMo fEk9+W+rID+ajYVvpvHwof3hhfS5A2bew+5+bK4D52Tj8HF55wtLYy+nt2io4G+/n4TS qhUPIJCQggJ/oOjK69PsSc8th4dcA56dhI6FMaYCrguQpG8kxl6Qye7hJ8o+XWNVY3GL ryT/1aepLzOiYyL3xgU18toO8R7qT6FoNqW05bYPgwp10NNT6tD3NbKI453OhDFkVa8v +nE4Q9s7mkMOtnT066y+TW1K5dsgkkM+n+4qzujysiCUnxVXkWj/QBqriOQYKWjl5W5k 2wMA== X-Gm-Message-State: AOAM530jgaD32grEg3Wgb7eBiFzAXj5Mei9jGzZLwBtMe0bwUlzi8Sw9 bnXxHWtzyrk6FVtQB4BObb+4rQ== X-Google-Smtp-Source: ABdhPJxS6074z7AcImZ0mTWjjPwnSfdggxQ3cLpGgJhxGX+f3Y0ZwPWxo+OWV7D3uUCa5TWFII9L6w== X-Received: by 2002:a05:6808:2185:b0:2d9:ebf0:fb66 with SMTP id be5-20020a056808218500b002d9ebf0fb66mr19072oib.69.1647882120958; Mon, 21 Mar 2022 10:02:00 -0700 (PDT) Received: from ?IPV6:2804:431:c7ca:2d55:f04a:67c7:cbf3:571d? ([2804:431:c7ca:2d55:f04a:67c7:cbf3:571d]) by smtp.gmail.com with ESMTPSA id z192-20020a4a49c9000000b003213bf4bf0csm6909277ooa.31.2022.03.21.10.01.59 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 21 Mar 2022 10:02:00 -0700 (PDT) Message-ID: Date: Mon, 21 Mar 2022 14:01:57 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 Subject: Re: rwlock different behavior on glibc 2.28 then on previous versions Content-Language: en-US To: David Mozes , Christian Hoff , Florian Weimer Cc: "libc-help@sourceware.org" , Carlos O'Donell References: <87czis1tgk.fsf@oldenburg.str.redhat.com> <8681b86f-963c-7c01-3d7a-9c4cb3da52c8@gmx.net> From: Adhemerval Zanella In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-help@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-help mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Mar 2022 17:02:03 -0000 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 > Sent: Monday, March 21, 2022 8:20 AM > To: David Mozes ; Florian Weimer > 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 >> 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 >> Sent: Friday, March 11, 2022 3:59 PM >> To: David Mozes >> 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