Anonymous | Login | 2024-03-29 08:46 UTC |
Main | My View | View Issues | Change Log | Docs |
Viewing Issue Simple Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||
ID | Category | Severity | Type | Date Submitted | Last Update | ||
0000720 | [1003.1(2008)/Issue 7] Base Definitions and Headers | Editorial | Clarification Requested | 2013-07-04 21:19 | 2019-06-10 08:55 | ||
Reporter | carlos | View Status | public | ||||
Assigned To | ajosey | ||||||
Priority | normal | Resolution | Accepted As Marked | ||||
Status | Closed | ||||||
Name | Carlos O'Donell | ||||||
Organization | Red Hat | ||||||
User Reference | |||||||
Section | pthread_rwlock_trywrlock / pthread_rwlock_wrlock (XSH, 3 System Interfaces) | ||||||
Page Number | 1682 | ||||||
Line Number | 53998 - 54005 | ||||||
Interp Status | Approved | ||||||
Final Accepted Text | Note: 0001767 | ||||||
Summary | 0000720: Can pthread_rwlock_wrlock be called recursively by the same thread? | ||||||
Description |
In the GNU C Library we have begun implementing support for hardware lock elision in our POSIX thread library. Lock elision brings with it certain restrictions on the behaviour of the interface and limits what locks can or can't be elided. For example because the lock is elided using hardware transactional memory it isn't possible to return EDEADLCK because that would involve taking the lock. This prevents elision for PTHREAD_MUTEX_NORMAL because that mutex type requires deadlock on relock. However, such behaviour is undefined with a PTHREAD_MUTEX_DEFAULT mutex and therefore we can elide all default-style mutex locks in an attempt to increase lock performance. As part of this work we are looking at the possibility of eliding rwlock locks and we have discovered some inconsistent language. Lines 54001 to 54005 say: ~~~ The pthread_rwlock_wrlock( ) function shall apply a write lock to the read-write lock referenced by rwlock. The calling thread acquires the write lock if no other thread (reader or writer) holds the read-write lock rwlock. Otherwise, the thread shall block until it can acquire the lock. The calling thread may deadlock if at the time the call is made it holds the read-write lock (whether a read or write lock). ~~~ The word "other" in the second sentence appears to imply that the lock may be taken recursively by the same thread without blocking. The use of "other" implying any thread but the present. If the lock can be taken recursively then the option to elide the lock is possible since deadlock detection is entirely optional for a rwlock i.e. "may fail" on line 54021. However, lines 53998 to 54000 say: ~~~ The pthread_rwlock_trywrlock( ) function shall apply a write lock like the pthread_rwlock_wrlock( ) function, with the exception that the function shall fail if any thread currently holds rwlock (for reading or writing). ~~~ The word "any" in the first sentence appears to imply that the lock can not be taken recursively. If the lock can't be taken recursively then it must block (deadlock) if a thread already holds the write lock and attempts to take it again. Such a requirement would make it impossible to elide the lock for these calls. Thus it seems like the language is inconsistent between the two interfaces. Either the write lock can't be taken recursively or it can. Likewise either the word "other" on line 54002 should be removed or the word "other" added after the word "any" on line 53999. Alternatively some other clarification can be provided. |
||||||
Desired Action |
One or the other: (a) No, an rwlock may not be locked recursively. * Remove the word "other" on line 54002. (b) Yes, an rwlock may be locked recursively. * Add the word "other" after "any" on line 53999. Note: I am open to the use of more detailed language that avoids the use of the word "other" to imply that the lock may be taken recursively. |
||||||
Tags | tc2-2008 | ||||||
Attached Files | |||||||
|
Relationships | |||||||
|
Notes | |
(0001664) andikleen2 (reporter) 2013-07-04 22:21 |
I would also like to see a clarification how "deadlock" is defined, e.g. with PTHREAD_MUTEX_NORMAL. Does deadlock require stopping on every execution? |
(0001665) geoffclare (manager) 2013-07-05 08:14 |
pthread_rwlock_trywrlock() fails if any thread holds the lock; it does not deadlock. |
(0001666) Konrad_Schwarz (reporter) 2013-07-05 09:16 edited on: 2013-07-05 11:26 |
Sorry for misusing this forum for TSX details, but if you were to use the Hardware Lock Elision feature, couldn't you detect a relock attempt, since the word that represents the spin lock is returned as modified locally (globally, it does not change). Was the decision to not use this feature based on Haswell's limit of 1 nested HLE? In any case, it seems wrong to make PTHREAD_MUTEX_DEFAULT locks higher performing than PTHREAD_MUTEX_NORMAL, as traditionally PTHREAD_MUTEX_NORMAL locks are the highest performing, and code was optimized to use them. That this code now needs to be revisited (to gain from TSX) is unfortunate. I think the original, perhaps half-baked, idea for PTHREAD_MUTEX_DEFAULT was to have an easy way of switching between PTHRAD_MUTEX_ERRORCHECK and PTHREAD_MUTEX_NORMAL. In any case, I think POSIX should re-think its requirement of having PTHREAD_MUTEX_NORMAL deadlock, and turn this into a "may" (perhaps in the next edition), since there is **no** real benefit for applications to have a guaranteed way of deadlocking. Konrad Schwarz |
(0001667) philip-guenther (reporter) 2013-07-05 09:46 |
Since pthread_rwlock_unlock()'s spec says this: If this function is called to release a write lock for this read-write lock object, the read-write lock object shall be put in the unlocked state. ...with no provision for it remaining locked, rwlocks cannot currently be recursive for write locking. I guess that does still open the possibility of returning zero without changing any state in the self-lock case. My gut reaction is that having self-lock attempts return zero sometimes (only when no contention, right?) will be (a) unexpected for developers, and (b) immensely frustrating to debug, because of the 'sometimes' aspect. Side note: in OpenBSD, PTHREAD_MUTEX_DEFAULT mutexes call abort() if you pthread_mutex_lock() a mutex you already hold or pthread_mutex_unlock() a mutex you don't hold...and that has turned up several locking bugs in open source projects like glib2, where code *simply failed to grab the right lock* and no one noticed because the default mutexes on Linux returned an ignorable error. The assumption that most code has been debugged completely and that therefore the default behavior should be to turn off the checks and run as fast as possible is a bad joke. |
(0001668) andikleen2 (reporter) 2013-07-05 21:29 |
Konrad Schwarz: I don't think this is the right forum to discuss TSX implementation details. I should only note that there are other architectures which support lock elision (e.g. the ones from IBM) that do not have HLE, so for them it would never be an option. I agree that deadlocks should be all "may" or be sufficiently undefined that the elision behavior is conforming. Philip Guenther: "sometimes" behavior is pretty much in the nature of parallelism, threading and locking, there is no way around it. |
(0001750) andikleen3 (reporter) 2013-08-21 23:49 |
Any decision on this? Is POSIX rwlocks planning to support lock elision or not? |
(0001751) geoffclare (manager) 2013-08-22 15:43 edited on: 2013-08-27 09:25 |
As I pointed out in Note: 0001665, the original problem report seems to be the result of misinterpreting the standard. The standard is clear that: pthread_rwlock_[try]rdlock() must be recursive if the calling thread already holds a read lock. pthread_rwlock_wrlock() must either deadlock or return an EDEADLK error if the calling thread already holds a read or write lock. pthread_rwlock_trywrlock() must return EBUSY if the calling thread already holds a read or write lock. Whether or not the standard should be changed to allow write locks to be recursive is another matter. |
(0001754) carlos (reporter) 2013-08-23 18:45 |
Geoff, This issue is not asking for POSIX to change what is or is not allowed for write locks. That is certainly another issue. However, it is indeed an issue of interpretation of the standard, and we would like clarification to prevent future misinterpretations. (1) pthread_rwlock_wrlock You write: ~~~ pthread_rwlock_wrlock() must either deadlock or return an EDEADLK error if the calling thread already holds a read or write lock. ~~~ Lines 54001 to 54005 say: ~~~ The pthread_rwlock_wrlock( ) function shall apply a write lock to the read-write lock referenced by rwlock. The calling thread acquires the write lock if no other thread (reader or writer) holds the read-write lock rwlock. Otherwise, the thread shall block until it can acquire the lock. The calling thread may deadlock if at the time the call is made it holds the read-write lock (whether a read or write lock). ~~~ The phrase `if no other thread' appears to have the meaning that the write lock may be taken recursively. That is to say that a single thread may call pthread_rwlock_wrlock as many times as it wishes and the standard allows for an implementation that does not deadlock and instead takes the lock recursively. If it can't be taken recursively then I'd like the following action item to be taken: (a) No, an rwlock may not be locked recursively. * Remove the word "other" on line 54002. e.g. Lines 54001 to 54005 would now say: ~~~ The pthread_rwlock_wrlock( ) function shall apply a write lock to the read-write lock referenced by rwlock. The calling thread acquires the write lock if no thread (reader or writer) holds the read-write lock rwlock. Otherwise, the thread shall block until it can acquire the lock. The calling thread may deadlock if at the time the call is made it holds the read-write lock (whether a read or write lock). ~~~ (2) pthread_rwlock_[try]rdlock You write: ~~~ pthread_rwlock_[try]rdlock() must be recursive if the calling thread already holds a read lock. ... pthread_rwlock_wrlock()[SIC] must return EBUSY if the calling thread already holds a read or write lock. ~~~ I assume you meant to write `pthread_rwlock_[try]rdlock' where you wrote `pthread_rwlock_wrlock()' since the latter should not return EBUSY. I don't see how these comments are relevant to the question of the behaviour of `pthread_rwlock_trywrlock'. This issue writes about `pthread_rwlock_trywrlock' in order to make it clear that there is a contradiction in the language used by the standard and that we are seeing clarification. In summary: I am looking for clarification on pthread_rwlock_wrlock(). It would appear that the consensus is that it can not be taken recursively and must deadlock or return EDEADLK. In that case I strongly suggest that the word `other' be removed from line 54002 to prevent future confusion. Andi, several others in the glibc community, and myself have all been confused by the wording of the standard and we would like to see clarification such that we can come to a conclusion on the lock elision work we are doing in glibc. This has immediate impact on Intel and IBM hardware, so we are interested in a resolution of this issue. |
(0001756) geoffclare (manager) 2013-08-27 09:48 |
(response to Note: 0001754) > (1) pthread_rwlock_wrlock > > You write: > ~~~ > pthread_rwlock_wrlock() must either deadlock or return an EDEADLK > error if the calling thread already holds a read or write lock. > ~~~ > > Lines 54001 to 54005 say: > ~~~ > The pthread_rwlock_wrlock( ) function shall apply a write lock to the > read-write lock referenced by rwlock. The calling thread acquires the > write lock if no other thread (reader or writer) holds the read-write > lock rwlock. Otherwise, the thread shall block until it can acquire > the lock. The calling thread may deadlock if at the time the call is > made it holds the read-write lock (whether a read or write lock). > ~~~ > > The phrase `if no other thread' appears to have the meaning that the > write lock may be taken recursively. That is to say that a single > thread may call pthread_rwlock_wrlock as many times as it wishes and > the standard allows for an implementation that does not deadlock and > instead takes the lock recursively. My take on that paragraph is that this part deals with the possibilities as regards whether or not other threads hold the lock: The pthread_rwlock_wrlock( ) function shall apply a write lock to the read-write lock referenced by rwlock. The calling thread acquires the write lock if no other thread (reader or writer) holds the read-write lock rwlock. Otherwise, the thread shall block until it can acquire the lock. and this part deals (together with the EDEADLK error) with the possibilities as regards whether or not the calling thread already holds the lock: The calling thread may deadlock if at the time the call is made it holds the read-write lock (whether a read or write lock). If you treat the first part as also applying when the calling thread holds the lock, then the "Otherwise" clause makes no sense as it would mandate deadlock for this case whereas the second part makes it optional. Also, if recursive write locks were allowed there would be a requirement, as there is for read locks, that "the application shall ensure that the thread performs matching unlocks", and the requirements for pthread_rwlock_trywrlock() would be different. > If it can't be taken recursively then I'd like the following action item to be taken: > > (a) No, an rwlock may not be locked recursively. > * Remove the word "other" on line 54002. > e.g. > Lines 54001 to 54005 would now say: > ~~~ > The pthread_rwlock_wrlock( ) function shall apply a write lock to the > read-write lock referenced by rwlock. The calling thread acquires the > write lock if no thread (reader or writer) holds the read-write lock > rwlock. Otherwise, the thread shall block until it can acquire the > lock. The calling thread may deadlock if at the time the call is made > it holds the read-write lock (whether a read or write lock). > ~~~ That still wouldn't be quite right. The "Otherwise" clause also needs to change, to: Otherwise, if another thread holds the read-write lock rwlock, the calling thread shall block until it can acquire the lock. in order not to conflict with the final sentence. > (2) pthread_rwlock_[try]rdlock > > You write: > ~~~ > pthread_rwlock_[try]rdlock() must be recursive if the calling > thread already holds a read lock. > > ... > > pthread_rwlock_wrlock()[SIC] must return EBUSY if the calling thread > already holds a read or write lock. > ~~~ > > I assume you meant to write `pthread_rwlock_[try]rdlock' where you wrote `pthread_rwlock_wrlock()' since the latter should not return EBUSY. > No, I meant to write pthread_rwlock_trywrlock(). I have corrected the comment. |
(0001757) torvald (reporter) 2013-08-28 08:12 edited on: 2013-08-28 08:13 |
Geoff, you write in Note: 0001756 > [...] this part deals (together with the EDEADLK error) with the > possibilities as regards whether or not the calling thread already holds > the lock: > > The calling thread may deadlock if at the time the call is > made it holds the read-write lock (whether a read or write lock). To me this seems to allow deadlocks, but not require them (i.e., it may be a recursive write lock, or may not be). However, ... > If you treat the first part as also applying when the calling thread > holds the lock, then the "Otherwise" clause makes no sense as it would > mandate deadlock for this case whereas the second part makes it optional. > > Also, if recursive write locks were allowed there would be a > requirement, as there is for read locks, that "the application shall > ensure that the thread performs matching unlocks", and the requirements > for pthread_rwlock_trywrlock() would be different. ... you seem to understand this sentence as requiring deadlocks (so it is not a recursive write lock). What is the correct interpretation? If implementations may choose to implement a recursive writelock for pthread_rwlock_wrlock, then for consistency reasons, I suppose it should be clarified that pthread_rwlock_trywrlock *may* return EBUSY, or may acquire recursively. Otherwise, if pthread_rwlock_wrlock is not recursive, then what about rephrasing as such: The calling thread acquires the write lock if no other thread (reader or writer) holds the read-write lock rwlock; otherwise, the thread shall block until it can acquire the lock. The calling thread shall deadlock if at the time the call is made it holds the read-write lock (whether a read or write lock). This would clarify the deadlock requirement. Also, it would clarify the context of "otherwise" by linking the first and second sentence with ";" (this is something we could apply irrespective of we resolve the issues around the last sentence). |
(0001759) geoffclare (manager) 2013-08-28 09:24 edited on: 2013-08-28 14:01 |
(response to Note: 0001757) I am sure that the intention is that pthread_rwlock_wrlock() must either deadlock or return EDEADLK when the calling thread already holds the lock. As I said in my previous note, if recursive locks were allowed, there would be a requirement about the need for matching unlocks, and the requirements for pthread_rwlock_trywrlock() would be different (it would be worded like pthread_rwlock_tryrdlock()). See also Philip Guenther's point (Note: 0001667) about pthread_rwlock_unlock(). The suggested text that says "The calling thread shall deadlock if at the time the call is made it holds the read-write lock" would imply that an EDEADLK error is not allowed, contradicting the ERRORS section. I think removing "other" and altering the "Otherwise" clause as per my previous note would suffice to fix the loophole that appears to allow pthread_rwlock_wrlock() to be recursive. |
(0001761) torvald (reporter) 2013-08-28 12:16 |
I agree about a plain "shall deadlock" disallowing the error code. Instead, what about saying "shall either deadlock or return an EDEADLK"? The plain "may deadlock" seems to have caused quite a bit of confusion, so perhaps it makes sense to be more verbose here. |
(0001762) geoffclare (manager) 2013-08-28 13:54 |
I like the "either ... or ..." suggestion, and suggest making it cover other deadlock conditions as well... Change: The calling thread acquires the write lock if no other thread (reader or writer) holds the read-write lock rwlock. Otherwise, the thread shall block until it can acquire the lock. The calling thread may deadlock if at the time the call is made it holds the read-write lock (whether a read or write lock). to: The calling thread shall acquire the write lock if no thread (reader or writer) holds the read-write lock rwlock. Otherwise, if another thread holds the read-write lock rwlock, the calling thread shall block until it can acquire the lock. If a deadlock condition occurs or the calling thread already owns the read-write lock for writing or reading, the call shall either deadlock or return EDEADLK. |
(0001767) geoffclare (manager) 2013-08-29 09:08 edited on: 2013-08-29 15:09 |
Interpretation response ------------------------ The standard clearly states that reader-writer locks held for reading are recursive and that reader-writer locks held for writing are not recursive, and conforming implementations must conform to this. Rationale: ------------- It is clear from the description of pthread_rwlock_[try]rdlock() and pthread_rwlock_unlock() that read locks are recursive. It is clear from the description of pthread_rwlock_trywrlock() and pthread_rwlock_unlock() that write locks are not recursive. The intention is that pthread_rwlock_wrlock() either deadlocks or returns EDEADLK if the caller already holds a write lock, but this is written as two separate "may" clauses (one in the DESCRIPTION and one in the ERRORS section), which creates a loophole that allows pthread_rwlock_wrlock() to succeed (as a no-op). However, this behavior would be of no use to applications. In particular, if an application tried to make matching pthread_rwlock_unlock() calls for two successful pthread_rwlock_wrlock() calls, the first unlock call would set the state to unlocked and the second unlock call would result in undefined behavior. Notes to the Editor (not part of this interpretation): ------------------------------------------------------- Make the change in Note: 0001762. |
(0001771) carlos (reporter) 2013-08-29 15:07 |
I'm happy with the new wording suggested in Note: 0001762. The goal here is to clarify the intent of the standard in order to allow an implementation to use elision or not use elision for locks of that particular type. |
(0001804) ajosey (manager) 2013-09-06 04:53 |
Interpretation Proposed 6 Sep 2013 |
(0001896) ajosey (manager) 2013-10-14 13:08 |
Interpretation approved 14 October 2013 |
Issue History | |||
Date Modified | Username | Field | Change |
2013-07-04 21:19 | carlos | New Issue | |
2013-07-04 21:19 | carlos | Status | New => Under Review |
2013-07-04 21:19 | carlos | Assigned To | => ajosey |
2013-07-04 21:19 | carlos | Name | => Carlos O'Donell |
2013-07-04 21:19 | carlos | Organization | => Red Hat |
2013-07-04 21:19 | carlos | Section | => pthread_rwlock_trywrlock / pthread_rwlock_wrlock (XSH, 3 System Interfaces) |
2013-07-04 21:19 | carlos | Page Number | => 1682 |
2013-07-04 21:19 | carlos | Line Number | => 53998 - 54005 |
2013-07-04 22:21 | andikleen2 | Note Added: 0001664 | |
2013-07-04 22:21 | andikleen2 | Issue Monitored: andikleen2 | |
2013-07-05 08:14 | geoffclare | Note Added: 0001665 | |
2013-07-05 09:16 | Konrad_Schwarz | Note Added: 0001666 | |
2013-07-05 09:46 | philip-guenther | Note Added: 0001667 | |
2013-07-05 11:26 | Konrad_Schwarz | Note Edited: 0001666 | |
2013-07-05 21:29 | andikleen2 | Note Added: 0001668 | |
2013-07-18 16:06 | eblake | Relationship added | related to 0000722 |
2013-08-21 23:49 | andikleen3 | Note Added: 0001750 | |
2013-08-22 15:43 | geoffclare | Note Added: 0001751 | |
2013-08-23 18:45 | carlos | Note Added: 0001754 | |
2013-08-27 09:25 | geoffclare | Note Edited: 0001751 | |
2013-08-27 09:48 | geoffclare | Note Added: 0001756 | |
2013-08-28 08:12 | torvald | Note Added: 0001757 | |
2013-08-28 08:13 | torvald | Note Edited: 0001757 | |
2013-08-28 09:24 | geoffclare | Note Added: 0001759 | |
2013-08-28 12:16 | torvald | Note Added: 0001761 | |
2013-08-28 13:54 | geoffclare | Note Added: 0001762 | |
2013-08-28 14:01 | geoffclare | Note Edited: 0001759 | |
2013-08-29 09:08 | geoffclare | Note Added: 0001767 | |
2013-08-29 09:10 | geoffclare | Note Edited: 0001767 | |
2013-08-29 15:07 | carlos | Note Added: 0001771 | |
2013-08-29 15:09 | geoffclare | Interp Status | => Pending |
2013-08-29 15:09 | geoffclare | Final Accepted Text | => Note: 0001767 |
2013-08-29 15:09 | geoffclare | Status | Under Review => Interpretation Required |
2013-08-29 15:09 | geoffclare | Resolution | Open => Accepted As Marked |
2013-08-29 15:09 | geoffclare | Note Edited: 0001767 | |
2013-08-29 15:09 | geoffclare | Tag Attached: tc2-2008 | |
2013-09-06 04:53 | ajosey | Interp Status | Pending => Proposed |
2013-09-06 04:53 | ajosey | Note Added: 0001804 | |
2013-10-14 13:08 | ajosey | Interp Status | Proposed => Approved |
2013-10-14 13:08 | ajosey | Note Added: 0001896 | |
2019-06-10 08:55 | agadmin | Status | Interpretation Required => Closed |
Mantis 1.1.6[^] Copyright © 2000 - 2008 Mantis Group |