Page MenuHomeFreeBSD

mips: fcmpset: do not spin on sc failure
ClosedPublic

Authored by kevans on Sep 29 2019, 2:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 22, 7:10 PM
Unknown Object (File)
Jan 1 2024, 3:16 AM
Unknown Object (File)
Jan 1 2024, 3:16 AM
Unknown Object (File)
Jan 1 2024, 3:16 AM
Unknown Object (File)
Jan 1 2024, 3:12 AM
Unknown Object (File)
Jan 1 2024, 3:12 AM
Unknown Object (File)
Jan 1 2024, 3:12 AM
Unknown Object (File)
Jan 1 2024, 2:58 AM
Subscribers
None

Details

Summary

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.

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
404–405

exit on failure (or abort)

405–407

This label should be '1' etc.

Re-label jumps and revise comment to be more specific

kib added inline comments.
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).

This revision is now accepted and ready to land.Sep 29 2019, 4:29 PM
kevans added inline comments.
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

Then you need a nop here. Otherwise I think the following instruction is used...
unless there's a special rule that a delay slot won't cross a label...

515

Same here.

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.

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.

This revision now requires review to proceed.Sep 30 2019, 2:20 AM
sys/mips/include/atomic.h
405

Actually this is an exit not only on failure.

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.

This revision is now accepted and ready to land.Sep 30 2019, 9:07 PM
This revision was automatically updated to reflect the committed changes.