rwlock for async application

Message ID AM9PR04MB829194D9C98E5708A168568CF11A9@AM9PR04MB8291.eurprd04.prod.outlook.com
State Not applicable
Headers
Series rwlock for async application |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

David Mozes March 25, 2022, 5:16 p.m. UTC
  Hi all,
I want to discuss some needs that I think need to address.
On async coding used mainly on networking and storage applications, how ever I believe on other applications as well, what we are doing is acquiring a lock on the application level, and then call to the OS/kernel for sending/writing
Callback instead of waitengin on blocking for the sending/writing to finish. After completing the OS, call the CB to release the transaction and the lock.
The problem is that the current glibc implumnation doesn't support taking and releasing the writer lock from different threads.

I think it needs to be address .
 Actually I did the folwing change:

index d3f36303bf..b1032cfa2a 100644


On the current unlock function and seems to working so far on heavy load  and 3k threads.

I believe it has some limitions and need farther review.and might be calling in different name,but I  think  it be good to have it.

Your thout?

Thx
David
  

Comments

Yann Droneaud March 25, 2022, 5:38 p.m. UTC | #1
Hi,

Le 25/03/2022 à 18:16, David Mozes a écrit :
> Hi all,
> I want to discuss some needs that I think need to address.
> On async coding used mainly on networking and storage applications, how ever I believe on other applications as well, what we are doing is acquiring a lock on the application level, and then call to the OS/kernel for sending/writing
> Callback instead of waitengin on blocking for the sending/writing to finish. After completing the OS, call the CB to release the transaction and the lock.
> The problem is that the current glibc implumnation doesn't support taking and releasing the writer lock from different threads.
>
> I think it needs to be address .
>   Actually I did the folwing change:


https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_unlock.html


"Results are undefined if the read-write lock /rwlock/ is not held by 
the calling thread"


Regards.
  
Adhemerval Zanella Netto March 29, 2022, 1:28 p.m. UTC | #2
On 25/03/2022 14:16, David Mozes wrote:
> Hi all,
> I want to discuss some needs that I think need to address.
> On async coding used mainly on networking and storage applications, how ever I believe on other applications as well, what we are doing is acquiring a lock on the application level, and then call to the OS/kernel for sending/writing
> C*allback* instead of waitengin on blocking for the sending/writing to finish. After completing the OS, call the CB to release the transaction and the lock.
> The problem is that the current glibc implumnation doesn’t support taking and releasing the writer lock from different threads.
>  
> I think it needs to be address .
> Actually I did the folwing change:
>  
> index d3f36303bf..b1032cfa2a 100644
> --- a/nptl/pthread_rwlock_unlock.c
> +++ b/nptl/pthread_rwlock_unlock.c
> @@ -36,8 +36,7 @@ __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
>       because nobody else can have stored this value.  Also, if we are a
>       reader, we will read from the wrunlock store with value 0 by the most
>       recent writer because that writer happens-before us.  */
> -  if (atomic_load_relaxed (&rwlock->__data.__cur_writer)
> -      == THREAD_GETMEM (THREAD_SELF, tid))
> +  if (atomic_load_relaxed (&rwlock->__data.__cur_writer))
>        __pthread_rwlock_wrunlock (rwlock);
>    else
>      __pthread_rwlock_rdunlock (rwlock);
>  
>  
> On the current unlock function and seems to working so far on heavy load  and 3k threads.
>  
> I believe it has some limitions and need farther review.and might be calling in different name,but I  think  it be good to have it.
>  
> Your thout?

Hi David,

As I tried to explain to you on libc-help, you can't blindly change the
algorithm without explaining why it is safe to do so. In this case it
means to explain why the test is not required and how only using 
__cur_writer is suffice to know whether the acquired lock is
a read or write.  It would be good to know also the performance 
implications, if any.

However the main issue, as pointed out my other, the POSIX standard is
explicit that it is undefined if the read-write lock rwlock is not held
by the calling thread. It means that if glibc allows, you are binding
its implementation to this specific semantic and making any program 
build on glibc potentially undefined if it is build for another libc. 
I tend to agree that enforcing write locks are held only for calling 
thread seems to work towards to make glibc more standard compatible.

One option might be to add a GNU extension to allow it, something that
PTHREAD_RWLOCK_ALLOW_WRITER_NOT_HELD_NP, which should be explicit opt-in
(instead of default).
  

Patch

--- a/nptl/pthread_rwlock_unlock.c
+++ b/nptl/pthread_rwlock_unlock.c
@@ -36,8 +36,7 @@  __pthread_rwlock_unlock (pthread_rwlock_t *rwlock)
      because nobody else can have stored this value.  Also, if we are a
      reader, we will read from the wrunlock store with value 0 by the most
      recent writer because that writer happens-before us.  */
-  if (atomic_load_relaxed (&rwlock->__data.__cur_writer)
-      == THREAD_GETMEM (THREAD_SELF, tid))
+  if (atomic_load_relaxed (&rwlock->__data.__cur_writer))
       __pthread_rwlock_wrunlock (rwlock);
   else
     __pthread_rwlock_rdunlock (rwlock);