For a pshare key, we can actually cache the last one as a fast path
instead of trapping into kernel directly. It makes sense as a lock and
unlock should be a pair on the same key and at the same time. For
example, lock(A) -> unlock(A) -> lock(B) -> unlock(B). Each unlock is
a hit and refresh later (50% hit).
For a single pshare lock, it is evne a 100% hit as the key never
changed.
For nested lock lock(A) -> lock(B) -> lock(C) -> unlock(C) ->
unlock(B) -> unlock(A). It only cache the last one. However, we should
really think about rewritting this logic as it is a pshare lock shared
by many processes and nested should be uncommon.
Details
Details
- Reviewers
markj kib - Commits
- rG7806832f23fb: libthr: Cache recent pshared_key
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 78705 Build 75588: arc lint + arc unit
Event Timeline
| lib/libthr/thread/thr_pshared.c | ||
|---|---|---|
| 111 | Why this load is acquire? What other accesses must occur after it for correctness? | |
| lib/libthr/thread/thr_pshared.c | ||
|---|---|---|
| 111 | Yes, it is no matter whether it sees old value or new value. But I am not sure whether TSan will be unhappy or not? I didn't test it. | |
| lib/libthr/thread/thr_pshared.c | ||
|---|---|---|
| 54 | Is there any reason not to use unsigned long, i.e., a 64-bit counter on LP64 systems? 32-bit counters are susceptible to ABA bugs, however unlikely. | |
| 111 | kib is asking why you used atomic_load_acq_32 instead of atomic_load_32. Why exactly do you need acquire semantics? | |
| lib/libthr/thread/thr_pshared.c | ||
|---|---|---|
| 111 | Oh I see. And yes, we don't need that as we don't use this as a check to write and read some other shared data. It is just a check to thread local stuff. | |
| lib/libthr/thread/thr_pshared.c | ||
|---|---|---|
| 104 | So why rel stil? | |