Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Noah Goldstein]

1) Do we need `__pthread_mutex_unlock_usercnt`?
__pthread_mutex_unlock_usercnt:
https://sourceware.org/git/?p=glibc.git;a=blob;f=nptl/pthread_cond_wait.c;h=20c348a50341518d75c31f3d18737ce12a1cbb00;hb=HEAD#l419

This just avoids decrementing `__nusers`. It seems to be we could
either 1) just increment `__nusers` in `pthread_cond_wait` before the
call or just allow `__nusers` to hit zero.

`__nusers` is only actually used in `pthread_mutex_destroy` to return
`EBUSY` if it's non-zero. As as calling `pthread_mutex_destroy` when
the lock has users it UB so just ignoring `__nusers` in
`pthread_cond_wait` should be fine.


NB: `__pthread_mutex_cond_lock` appears to be needed as there is logic
around assuming futex waiters.



2) Are people in favor of making it so that
`__pthread_mutex_unlock_usercnt` / `pthread_mutex_cond_lock` go
through the PLT so that researchers trying to replace `pthread_mutex`
in applications rely solely on LD_PRELOAD?

Re: Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Florian Weimer]

* Noah Goldstein via Libc-alpha:

> 1) Do we need `__pthread_mutex_unlock_usercnt`?

POSIX says:

| Attempting to destroy a locked mutex, or a mutex that another thread
| is attempting to lock, or a mutex that is being used in a
| pthread_cond_timedwait() or pthread_cond_wait() call by another
| thread, results in undefined behavior.

This does not change for PTHREAD_MUTEX_ERRORCHECK mutexes, so I think we
can remove usercnt.

> 2) Are people in favor of making it so that
> `__pthread_mutex_unlock_usercnt` / `pthread_mutex_cond_lock` go
> through the PLT so that researchers trying to replace `pthread_mutex`
> in applications rely solely on LD_PRELOAD?

Which functions do you mean exactly?

I think most internal locks are directly implemented on top of futexes,
so I don't think this would make much of a difference.

Thanks,
Florian

Re: Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Noah Goldstein]

On Thu, May 26, 2022 at 3:09 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Noah Goldstein via Libc-alpha:
>
> > 1) Do we need `__pthread_mutex_unlock_usercnt`?
>
> POSIX says:
>
> | Attempting to destroy a locked mutex, or a mutex that another thread
> | is attempting to lock, or a mutex that is being used in a
> | pthread_cond_timedwait() or pthread_cond_wait() call by another
> | thread, results in undefined behavior.
>
> This does not change for PTHREAD_MUTEX_ERRORCHECK mutexes, so I think we
> can remove usercnt.

Open to a patch removing the `__pthread_mutex_unlock_usercnt` function?
>
> > 2) Are people in favor of making it so that
> > `__pthread_mutex_unlock_usercnt` / `pthread_mutex_cond_lock` go
> > through the PLT so that researchers trying to replace `pthread_mutex`
> > in applications rely solely on LD_PRELOAD?
>
> Which functions do you mean exactly?
>
> I think most internal locks are directly implemented on top of futexes,
> so I don't think this would make much of a difference.

Can we make it so that
`__pthread_mutex_unlock_usercnt` and `__pthread_mutex_cond_lock`
used internally by `pthread_cond_{timed}wait`  go through the PLT so
that using LD_PRELOAD to entirely replace pthread_mutex in an application
is feasible. Think this would be pretty useful to lock researchers / people
that want to roll custom locks that are ABI compatible with
pthread_mutex/PTHREAD_MUTEX_INITIALIZER.

For reference this is how people seem to do it now:
https://github.com/multicore-locks/litl/blob/master/src/clh.c#L68

which essentially duplicates the pthread_mutex.

>
> Thanks,
> Florian
>

Re: Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Florian Weimer]

* Noah Goldstein:

> On Thu, May 26, 2022 at 3:09 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Noah Goldstein via Libc-alpha:
>>
>> > 1) Do we need `__pthread_mutex_unlock_usercnt`?
>>
>> POSIX says:
>>
>> | Attempting to destroy a locked mutex, or a mutex that another thread
>> | is attempting to lock, or a mutex that is being used in a
>> | pthread_cond_timedwait() or pthread_cond_wait() call by another
>> | thread, results in undefined behavior.
>>
>> This does not change for PTHREAD_MUTEX_ERRORCHECK mutexes, so I think we
>> can remove usercnt.
>
> Open to a patch removing the `__pthread_mutex_unlock_usercnt`
> function?

And for making that field unused, yes.

>> > 2) Are people in favor of making it so that
>> > `__pthread_mutex_unlock_usercnt` / `pthread_mutex_cond_lock` go
>> > through the PLT so that researchers trying to replace `pthread_mutex`
>> > in applications rely solely on LD_PRELOAD?
>>
>> Which functions do you mean exactly?
>>
>> I think most internal locks are directly implemented on top of futexes,
>> so I don't think this would make much of a difference.
>
> Can we make it so that
> `__pthread_mutex_unlock_usercnt` and `__pthread_mutex_cond_lock`
> used internally by `pthread_cond_{timed}wait`  go through the PLT so
> that using LD_PRELOAD to entirely replace pthread_mutex in an application
> is feasible. Think this would be pretty useful to lock researchers / people
> that want to roll custom locks that are ABI compatible with
> pthread_mutex/PTHREAD_MUTEX_INITIALIZER.

Why can't they interpose the condition variable implementation as well?

Thanks,
Florian

Re: Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Noah Goldstein]

On Thu, May 26, 2022 at 11:17 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Noah Goldstein:
>
> > On Thu, May 26, 2022 at 3:09 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * Noah Goldstein via Libc-alpha:
> >>
> >> > 1) Do we need `__pthread_mutex_unlock_usercnt`?
> >>
> >> POSIX says:
> >>
> >> | Attempting to destroy a locked mutex, or a mutex that another thread
> >> | is attempting to lock, or a mutex that is being used in a
> >> | pthread_cond_timedwait() or pthread_cond_wait() call by another
> >> | thread, results in undefined behavior.
> >>
> >> This does not change for PTHREAD_MUTEX_ERRORCHECK mutexes, so I think we
> >> can remove usercnt.
> >
> > Open to a patch removing the `__pthread_mutex_unlock_usercnt`
> > function?
>
> And for making that field unused, yes.
>
> >> > 2) Are people in favor of making it so that
> >> > `__pthread_mutex_unlock_usercnt` / `pthread_mutex_cond_lock` go
> >> > through the PLT so that researchers trying to replace `pthread_mutex`
> >> > in applications rely solely on LD_PRELOAD?
> >>
> >> Which functions do you mean exactly?
> >>
> >> I think most internal locks are directly implemented on top of futexes,
> >> so I don't think this would make much of a difference.
> >
> > Can we make it so that
> > `__pthread_mutex_unlock_usercnt` and `__pthread_mutex_cond_lock`
> > used internally by `pthread_cond_{timed}wait`  go through the PLT so
> > that using LD_PRELOAD to entirely replace pthread_mutex in an application
> > is feasible. Think this would be pretty useful to lock researchers / people
> > that want to roll custom locks that are ABI compatible with
> > pthread_mutex/PTHREAD_MUTEX_INITIALIZER.
>
> Why can't they interpose the condition variable implementation as well?

Either work, but obviously easier to just do mutex. Think less
interest/performance
is focussed on condition variables.

Is your feeling since it's doable now with a little extra work is not worth
the perf hit for normal users?
>
> Thanks,
> Florian
>

Re: Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Florian Weimer]

* Noah Goldstein:

> Either work, but obviously easier to just do mutex. Think less
> interest/performance is focussed on condition variables.
>
> Is your feeling since it's doable now with a little extra work is not
> worth the perf hit for normal users?

I think it creates an expectation about interface compatibility that
might not hold in the future.  Interposing all synchronization
interfaces at the same time seems more future-proof.

Thanks,
Florian

Re: Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Noah Goldstein]

On Thu, May 26, 2022 at 11:17 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Noah Goldstein:
>
> > On Thu, May 26, 2022 at 3:09 AM Florian Weimer <fweimer@redhat.com> wrote:
> >>
> >> * Noah Goldstein via Libc-alpha:
> >>
> >> > 1) Do we need `__pthread_mutex_unlock_usercnt`?
> >>
> >> POSIX says:
> >>
> >> | Attempting to destroy a locked mutex, or a mutex that another thread
> >> | is attempting to lock, or a mutex that is being used in a
> >> | pthread_cond_timedwait() or pthread_cond_wait() call by another
> >> | thread, results in undefined behavior.
> >>
> >> This does not change for PTHREAD_MUTEX_ERRORCHECK mutexes, so I think we
> >> can remove usercnt.
> >
> > Open to a patch removing the `__pthread_mutex_unlock_usercnt`
> > function?
>
> And for making that field unused, yes.

We have a fair number of tests to check that destroy on a "used" lock
returns EBUSY.

User code might have started to rely on it. Maybe best to keep or you
think still go ahead?

>
> >> > 2) Are people in favor of making it so that
> >> > `__pthread_mutex_unlock_usercnt` / `pthread_mutex_cond_lock` go
> >> > through the PLT so that researchers trying to replace `pthread_mutex`
> >> > in applications rely solely on LD_PRELOAD?
> >>
> >> Which functions do you mean exactly?
> >>
> >> I think most internal locks are directly implemented on top of futexes,
> >> so I don't think this would make much of a difference.
> >
> > Can we make it so that
> > `__pthread_mutex_unlock_usercnt` and `__pthread_mutex_cond_lock`
> > used internally by `pthread_cond_{timed}wait`  go through the PLT so
> > that using LD_PRELOAD to entirely replace pthread_mutex in an application
> > is feasible. Think this would be pretty useful to lock researchers / people
> > that want to roll custom locks that are ABI compatible with
> > pthread_mutex/PTHREAD_MUTEX_INITIALIZER.
>
> Why can't they interpose the condition variable implementation as well?
>
> Thanks,
> Florian
>

Re: Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Florian Weimer]

* Noah Goldstein:

> On Thu, May 26, 2022 at 11:17 AM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Noah Goldstein:
>>
>> > On Thu, May 26, 2022 at 3:09 AM Florian Weimer <fweimer@redhat.com> wrote:
>> >>
>> >> * Noah Goldstein via Libc-alpha:
>> >>
>> >> > 1) Do we need `__pthread_mutex_unlock_usercnt`?
>> >>
>> >> POSIX says:
>> >>
>> >> | Attempting to destroy a locked mutex, or a mutex that another thread
>> >> | is attempting to lock, or a mutex that is being used in a
>> >> | pthread_cond_timedwait() or pthread_cond_wait() call by another
>> >> | thread, results in undefined behavior.
>> >>
>> >> This does not change for PTHREAD_MUTEX_ERRORCHECK mutexes, so I think we
>> >> can remove usercnt.
>> >
>> > Open to a patch removing the `__pthread_mutex_unlock_usercnt`
>> > function?
>>
>> And for making that field unused, yes.
>
> We have a fair number of tests to check that destroy on a "used" lock
> returns EBUSY.

Yes, I think those are invalid and needs to be adjusted.

LTP probably needs updating as well.

> User code might have started to rely on it. Maybe best to keep or you
> think still go ahead?

For robust mutexes, maybe we should remove a locked mutex from the
current thread's locked list instead.  Not doing so causes memory
corruption at a distance that could be difficult to debug.  That would
be an improvement over the current situation, I think.

Thanks,
Florian

Re: Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Yann Droneaud]

Le 26/05/2022 à 18:31, Florian Weimer via Libc-alpha a écrit :
> * Noah Goldstein:
>
>> Either work, but obviously easier to just do mutex. Think less
>> interest/performance is focussed on condition variables.
>>
>> Is your feeling since it's doable now with a little extra work is not
>> worth the perf hit for normal users?
> I think it creates an expectation about interface compatibility that
> might not hold in the future.  Interposing all synchronization
> interfaces at the same time seems more future-proof.


pthread_rwlock, pthread_barrier, and pthread_once too even if they don't 
use a pthread_mutex ?


Regards.

-- 

Yann Droneaud

OPTEYA

Re: Questions regarding pthread mutex lock/unlock functions in pthread_cond_wait?

[Carlos O'Donell]

On 5/26/22 12:31, Florian Weimer via Libc-alpha wrote:
> * Noah Goldstein:
> 
>> Either work, but obviously easier to just do mutex. Think less
>> interest/performance is focussed on condition variables.
>>
>> Is your feeling since it's doable now with a little extra work is not
>> worth the perf hit for normal users?
> 
> I think it creates an expectation about interface compatibility that
> might not hold in the future.  Interposing all synchronization
> interfaces at the same time seems more future-proof.

I agree with Florian.

This is similar to the malloc scenario where users wanted to interpose only
morecore to implement malloc on top of another sources of pages.

It isn't just the performance hit, it is about the expectation we set.

-- 
Cheers,
Carlos.