Linux-man Archive on lore.kernel.org
 help / color / Atom feed
From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: Carlos O'Donell <carlos@redhat.com>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>
Cc: mtk.manpages@gmail.com, linux-man <linux-man@vger.kernel.org>
Subject: Re: [PATCH] pthread_rwlockattr_setkind_np.3: PTHREAD_RWLOCK_PREFER_WRITER_NP
Date: Fri, 17 Jul 2020 10:05:38 +0200
Message-ID: <a3b2e2e2-1738-9098-cbf6-872e6d2a2a54@gmail.com> (raw)
In-Reply-To: <209e4a6d-285d-7b3b-4acd-340e74da2863@redhat.com>

Hello Carlos, Kumas,

On 7/16/20 9:50 PM, Carlos O'Donell wrote:
> On 7/15/20 3:50 PM, Michael Kerrisk (man-pages) wrote:
>> On Fri, 26 Jun 2020 at 10:54, Michael Kerrisk (man-pages)
>> <mtk.manpages@gmail.com> wrote:
>>>
>>> Hi Carlos,
>>>
>>> Could you comment here, as this was your text in pthread_rwlockattr_setkind_np(3)?
>>>
>>> On 6/25/20 2:32 PM, Kumar Kartikeya Dwivedi wrote:
>>>> Hi,
>>>> In pthread_rwlockattr_setkind_np(3), the explanation for
>>>> PTHREAD_RWLOCK_PREFER_WRITER_NP reads:
>>>>
>>>>> This is ignored by glibc because the POSIX requirement to support
>>>>> recursive writer locks would cause this option to create trivial
>>>>> deadlocks;
>>>>
>>>> I think this should be "reader locks" instead, since it is
>>>> undefined[1] for a thread holding a write lock to call
>>>> pthread_rwlock_wrlock(3) again (glibc returns EDEADLK, musl simply
>>>> deadlocks). There's no such requirement in POSIX either. Please let me
>>>> know if I'm missing something.
>>>>
>>>> [1]: https://pubs.opengroup.org/onlinepubs/007908799/xsh/pthread_rwlock_wrlock.html
> 
> I agree with Kumar, and the comment I provided in commit 0d255e74c0 lines up
> with my intent and Kumar's requested correction (which is why it's always important
> to comment things to provide intent for the implementation!).
> 
> 8< --- 8< --- 8<
> Clarify that it is recursive read locks on the read-write lock that
> make it difficult to implement PTHREAD_RWLOCK_PREFER_WRITER_NP.
> 
> Update the libc-alpha URL and provide the URL to the POSIX wording
> that is quoted in the comment.
> 
> Signed-off-by: Carlos O'Donell <carlos@redhat.com>

Thanks for the patch, Carlos. Applied.

Thanks for the report, Kumar!

Cheers,

Michael
> 
> diff --git a/man3/pthread_rwlockattr_setkind_np.3 b/man3/pthread_rwlockattr_setkind_np.3
> index b381972dc..a91c43552 100644
> --- a/man3/pthread_rwlockattr_setkind_np.3
> +++ b/man3/pthread_rwlockattr_setkind_np.3
> @@ -80,7 +80,7 @@ starved.
>  This is intended as the write lock analog of
>  .BR PTHREAD_RWLOCK_PREFER_READER_NP .
>  This is ignored by glibc because the POSIX requirement to support
> -recursive writer locks would cause this option to create trivial
> +recursive read locks would cause this option to create trivial
>  deadlocks; instead use
>  .B PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
>  which ensures the application developer will not take recursive
> @@ -102,7 +102,8 @@ read locks thus avoiding deadlocks.
>  .\" the writers to acquire and release the lock, and the writers will be
>  .\" suspended waiting for every existing read lock to be released.
>  .\" ---
> -.\" http://sources.redhat.com/ml/libc-alpha/2000-01/msg00055.html
> +.\" https://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_rdlock.html
> +.\" https://sourceware.org/legacy-ml/libc-alpha/2000-01/msg00055.html
>  .\" https://sourceware.org/bugzilla/show_bug.cgi?id=7057
>  .TP
>  .B PTHREAD_RWLOCK_PREFER_WRITER_NONRECURSIVE_NP
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

      reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAP01T764kz5T+m+8sV0o30enBL1TagF7RQSjU0XcVJ0PvL0PTg@mail.gmail.com>
2020-06-26  8:54 ` POSIX writer locks can't be recursive Michael Kerrisk (man-pages)
2020-07-15 19:50   ` Michael Kerrisk (man-pages)
2020-07-16 19:50     ` [PATCH] pthread_rwlockattr_setkind_np.3: PTHREAD_RWLOCK_PREFER_WRITER_NP Carlos O'Donell
2020-07-17  8:05       ` Michael Kerrisk (man-pages) [this message]

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=a3b2e2e2-1738-9098-cbf6-872e6d2a2a54@gmail.com \
    --to=mtk.manpages@gmail.com \
    --cc=carlos@redhat.com \
    --cc=linux-man@vger.kernel.org \
    --cc=memxor@gmail.com \
    /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

Linux-man Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-man/0 linux-man/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-man linux-man/ https://lore.kernel.org/linux-man \
		linux-man@vger.kernel.org
	public-inbox-index linux-man

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-man


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git