x86 atomics: use lock prefix unconditionally
ClosedPublic
Actions

Authored by kib on Feb 3 2022, 9:59 AM.

Details

Reviewers

ehem_freebsd_m5p.com
royger
jhb
markj
imp

Commits

rG9596b349bb57: x86 atomic.h: remove obsoleted comment
rG9c0b759bf9b5: x86 atomics: use lock prefix unconditionally
rGcbf999e75d61: x86 atomic.h: cleanup comments for preprocessor directives

Summary

Atomics have significant other use besides providing in-system
primitives for safe memory updates.  They are used for implementing
communication with out of system software or hardware following some
protocols.

For instance, even UP kernel might require a protocol using atomics to
communicate with the software-emulated device on SMP hypervisor.  Or
real hardware might need atomic accesses as part of the proper
management protocol.

Another point is that UP configurations on x86 are extinct, so slight
performance hit by unconditionally use proper atomics is not important.
It is compensated by less code clutter, which in fact improves the
UP/i386 lifetime expectations.

Requested by:   Elliott Mitchell <ehem+freebsd@m5p.com>

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

kib created this revision.Feb 3 2022, 9:59 AM

Herald added a subscriber: imp. · View Herald TranscriptFeb 3 2022, 9:59 AM

kib requested review of this revision.Feb 3 2022, 9:59 AM

kib edited the summary of this revision. (Show Details)Feb 3 2022, 10:06 AM

s/read hardware/real hardware/ in the commit message I think.

This revision is now accepted and ready to land.Feb 3 2022, 10:21 AM

kib edited the summary of this revision. (Show Details)Feb 3 2022, 10:22 AM

markj accepted this revision.Feb 3 2022, 2:11 PM

One less special case for klds. Yea!

The two comments also need to be removed or adjusted, but that can be done during commit. Yet another generation of hardware/software passes on.

sys/amd64/include/atomic.h
105–112	This makes the second sentence of the first paragraph no longer true. The second paragraph became untrue at f4b3640475cec9299517289b7e8731ae26997cfc. I would tend to simply purge the comment, though it could also be adjusted.
sys/i386/include/atomic.h
99–106	Same situation with the identical comment here.

Remove obsoleted comment.

This revision now requires review to proceed.Feb 3 2022, 5:48 PM

ehem_freebsd_m5p.com accepted this revision.Feb 3 2022, 5:56 PM

This revision is now accepted and ready to land.Feb 3 2022, 5:56 PM

ehem_freebsd_m5p.com mentioned this in D30997: xen/arm64: add sys/arm64/include/xen/synch_bitops.h.Feb 3 2022, 6:06 PM

markj accepted this revision.Feb 3 2022, 6:44 PM

jhb accepted this revision.Feb 3 2022, 7:31 PM

jhb added inline comments.

sys/amd64/include/atomic.h
164	Maybe keep the space after the ; so that if anyone looks at the generated assembly it is 'lock; and' vs 'lock;and'? (In examples below you use a space in, e.g. 'lock; cmpxchg'.) Hmm, looks like you did keep the space in i386?

Keep space after lock; on amd64

This revision now requires review to proceed.Feb 3 2022, 8:30 PM

I was also a bit worried about the losing the space after the "lock;", just I thought removing it could be justified (didn't realize it was still there for i386). Hopefully all the end of line backslashes still line up. I'm assuming that is Phabricator not rendering things correctly.

This revision is now accepted and ready to land.Feb 3 2022, 10:02 PM

Closed by commit rGcbf999e75d61: x86 atomic.h: cleanup comments for preprocessor directives (authored by kib). · Explain WhyFeb 4 2022, 12:02 PM

This revision was automatically updated to reflect the committed changes.

kib added a commit: rGcbf999e75d61: x86 atomic.h: cleanup comments for preprocessor directives.

kib added a commit: rG9c0b759bf9b5: x86 atomics: use lock prefix unconditionally.

kib added a commit: rG9596b349bb57: x86 atomic.h: remove obsoleted comment.