For ll/sc architectures, atomic(9) allows failure modes where *old == val due to write failure and callers should compensate for this. Do not retry on failure, just leave 0 in ret and fail the operation if we couldn't sc it.
Details
Details
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 26782
Event Timeline
| sys/mips/include/atomic.h | ||
|---|---|---|
| 405–407 | So this uses the delay slot to provide the old value, right ? I think it deserves a comment. (Not that the old code did explained it). | |
| sys/mips/include/atomic.h | ||
|---|---|---|
| 405–407 | I do not believe so- in the comparison failure case, we've jumped to the sw below and provided. If the comparison didn't fail, we've left *cmpval untouched. | |
| sys/mips/include/atomic.h | ||
|---|---|---|
| 405–407 | Regardless of whether it can cross a label or not (which would seem odd, my impression was labels aren't actually compiled in but rather just the location noted for replacement in the underlying asm... but I haven't bothered checking this), it would likely be better to provide nop to be explicit. | |
| sys/mips/include/atomic.h | ||
|---|---|---|
| 405–407 | yea... The assembler is 'smart' and helps one manage the delay slots and sometimes inserts NOPs and other times doesn't. It's been 20ish years since I did MIPS assembler enough to have that swapped into my brain all the special times things would be written, and when they wouldn't... explicit nop would make it clear. | |
Comment Actions
Add nops to document that we're expecting no delay slot interactions to be happening here...
I would assume the assembler's implicitly adding this nop anyways based on the old version that we're replacing, but explicit is better than implicit.
| sys/mips/include/atomic.h | ||
|---|---|---|
| 405 | Actually this is an exit not only on failure. | |
Comment Actions
Revise incorrect comment, drop a more complete comment describing that it's intentionally leaving *cmpval untouched and a pointer to the documentation that says it's ok.