nptl: pthread_rwlock_rdlock return in low priority
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
hi, I have noticed reader will return directly on fast-path in pthread_rwlock_common.c
> /* We have registered as a reader, so if we are in a read phase, we have
> acquired a read lock. This is also the reader--reader fast-path.
> Even if there is a primary writer, we just return. If writers are to
> be preferred and we are the only active reader, we could try to enter a
> write phase to let the writer proceed. This would be okay because we
> cannot have acquired the lock previously as a reader (which could result
> in deadlock if we would wait for the primary writer to run). However,
> this seems to be a corner case and handling it specially not be worth the
> complexity. */
> if (__glibc_likely ((r & PTHREAD_RWLOCK_WRPHASE) == 0))
> return 0;
However, there is a situation:
main, thread_wr, thread_rd.
SCHED_FIFO priority:
main > thread_wr > thread_rd
main first acquires read lock, then create thread_wr which will block on the lock.
Next, main creates thread_rd. this thread will acquires read lock on fast-path even
though it has a lower priority compared to thread_wr.
You can get demo from the following repository:
https://github.com/emscripten-core/posixtestsuite.git
./conformance/interfaces/pthread_rwlock_rdlock/2-1.c
According to "man -M man-pages-posix-2017/ 3p pthread_rwlock_rdlock"
> DESCRIPTION
> The pthread_rwlock_rdlock() function shall apply a read lock to the
> read-write lock referenced by rwlock. The calling thread acquires the
> read lock if a writer does not hold the lock and there are no
> writers blocked on the lock.
>
> If the Thread Execution Scheduling option is supported, and the
> threads involved in the lock are executing with the scheduling
> policies SCHED_FIFO or SCHED_RR, the calling thread shall not acquire
> the lock if a writer holds the lock or if writers of higher or equal
> priority are blocked on the lock; other‐ wise, the calling thread
> shall acquire the lock.
I was wondering that whether this fast-path is reasonable, and whether
this posix standard should be enforced.
Thanks
Signed-off-by: abushwang <abushwangs@gmail.com>
---
nptl/pthread_rwlock_common.c | 1 +
1 file changed, 1 insertion(+)
Comments
On Tue, 2023-03-07 at 11:50 +0800, abushwang via Libc-alpha wrote:
> Signed-off-by: abushwang <abushwangs@gmail.com>
> ---
> nptl/pthread_rwlock_common.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
> index 5266a00ed1..35b00fc14f 100644
> --- a/nptl/pthread_rwlock_common.c
> +++ b/nptl/pthread_rwlock_common.c
> @@ -389,6 +389,7 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
> complexity. */
> if (__glibc_likely ((r & PTHREAD_RWLOCK_WRPHASE) == 0))
> return 0;
> +
I don't think simply adding an empty line can fix anything.
> /* Otherwise, if we were in a write phase (states #6 or #8), we must wait
> for explicit hand-over of the read phase; the only exception is if we
> can start a read phase if there is no primary writer currently. */
> --
> 2.36.1
ignore this empty line, i just want send this mail
Xi Ruoyao <xry111@xry111.site> 于2023年3月7日周二 16:40写道:
> On Tue, 2023-03-07 at 11:50 +0800, abushwang via Libc-alpha wrote:
> > Signed-off-by: abushwang <abushwangs@gmail.com>
> > ---
> > nptl/pthread_rwlock_common.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
> > index 5266a00ed1..35b00fc14f 100644
> > --- a/nptl/pthread_rwlock_common.c
> > +++ b/nptl/pthread_rwlock_common.c
> > @@ -389,6 +389,7 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t
> *rwlock, clockid_t clockid,
> > complexity. */
> > if (__glibc_likely ((r & PTHREAD_RWLOCK_WRPHASE) == 0))
> > return 0;
> > +
>
> I don't think simply adding an empty line can fix anything.
>
> > /* Otherwise, if we were in a write phase (states #6 or #8), we must
> wait
> > for explicit hand-over of the read phase; the only exception is if
> we
> > can start a read phase if there is no primary writer currently. */
> > --
> > 2.36.1
>
> --
> Xi Ruoyao <xry111@xry111.site>
> School of Aerospace Science and Technology, Xidian University
>
* abushwang via Libc-alpha:
> diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
> index 5266a00ed1..35b00fc14f 100644
> --- a/nptl/pthread_rwlock_common.c
> +++ b/nptl/pthread_rwlock_common.c
> @@ -389,6 +389,7 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
> complexity. */
> if (__glibc_likely ((r & PTHREAD_RWLOCK_WRPHASE) == 0))
> return 0;
> +
> /* Otherwise, if we were in a write phase (states #6 or #8), we must wait
> for explicit hand-over of the read phase; the only exception is if we
> can start a read phase if there is no primary writer currently. */
This does not seem to be the right patch?
Thanks,
Florian
Actually, i can not send mail without any change by git sendmail, so I add
a empty line.
This is just a mail for advisory
Florian Weimer <fweimer@redhat.com> 于2023年3月7日周二 16:56写道:
> * abushwang via Libc-alpha:
>
> > diff --git a/nptl/pthread_rwlock_common.c b/nptl/pthread_rwlock_common.c
> > index 5266a00ed1..35b00fc14f 100644
> > --- a/nptl/pthread_rwlock_common.c
> > +++ b/nptl/pthread_rwlock_common.c
> > @@ -389,6 +389,7 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t
> *rwlock, clockid_t clockid,
> > complexity. */
> > if (__glibc_likely ((r & PTHREAD_RWLOCK_WRPHASE) == 0))
> > return 0;
> > +
> > /* Otherwise, if we were in a write phase (states #6 or #8), we must
> wait
> > for explicit hand-over of the read phase; the only exception is if
> we
> > can start a read phase if there is no primary writer currently. */
>
> This does not seem to be the right patch?
>
> Thanks,
> Florian
>
>
On Tue, 2023-03-07 at 17:06 +0800, abush wang via Libc-alpha wrote:
> Actually, i can not send mail without any change by git sendmail, so I
> add
> a empty line.
> This is just a mail for advisory
Don't use git send-email then. It's a utility to send patches as mails,
not ordinary mails.
If you need a CLI tool for email use sendmail or something.
>
> Florian Weimer <fweimer@redhat.com> 于2023年3月7日周二 16:56写道:
>
> > * abushwang via Libc-alpha:
> >
> > > diff --git a/nptl/pthread_rwlock_common.c
> > > b/nptl/pthread_rwlock_common.c
> > > index 5266a00ed1..35b00fc14f 100644
> > > --- a/nptl/pthread_rwlock_common.c
> > > +++ b/nptl/pthread_rwlock_common.c
> > > @@ -389,6 +389,7 @@ __pthread_rwlock_rdlock_full64
> > > (pthread_rwlock_t
> > *rwlock, clockid_t clockid,
> > > complexity. */
> > > if (__glibc_likely ((r & PTHREAD_RWLOCK_WRPHASE) == 0))
> > > return 0;
> > > +
> > > /* Otherwise, if we were in a write phase (states #6 or #8), we
> > > must
> > wait
> > > for explicit hand-over of the read phase; the only exception
> > > is if
> > we
> > > can start a read phase if there is no primary writer
> > > currently. */
> >
> > This does not seem to be the right patch?
> >
> > Thanks,
> > Florian
> >
> >
@@ -389,6 +389,7 @@ __pthread_rwlock_rdlock_full64 (pthread_rwlock_t *rwlock, clockid_t clockid,
complexity. */
if (__glibc_likely ((r & PTHREAD_RWLOCK_WRPHASE) == 0))
return 0;
+
/* Otherwise, if we were in a write phase (states #6 or #8), we must wait
for explicit hand-over of the read phase; the only exception is if we
can start a read phase if there is no primary writer currently. */