Page MenuHomeFreeBSD
Differential D35991

wg: Remove some atomics for kp pointers.
Needs ReviewPublic
Actions

Authored by jhb on Jul 29 2022, 4:57 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Oct 1, 4:35 AM
Unknown Object (File)
Tue, Sep 29, 2:23 PM
Unknown Object (File)
Sat, Sep 26, 11:55 AM
Unknown Object (File)
Sun, Sep 13, 11:53 AM
Unknown Object (File)
Fri, Sep 11, 5:20 AM
Unknown Object (File)
Fri, Sep 11, 3:27 AM
Unknown Object (File)
Tue, Sep 8, 11:06 PM
Unknown Object (File)
Tue, Sep 8, 2:02 PM
View All Files
Subscribers
emaste
imp

Details

Reviewers
markj
Summary

Atomics are not needed for accesses synchronized by a mutex.

There is one unlocked read of the pointer for which the atomic
load doesn't do anything (in terms of limiting races).

Atomics remain for accesses under the NET_EPOCH to provide
READ_ONCE semantics.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 46683
Build 43572: arc lint + arc unit

Event Timeline

jhb created this revision.Jul 29 2022, 4:57 PM
Herald added a subscriber: imp. · View Herald TranscriptJul 29 2022, 4:57 PM
jhb requested review of this revision.Jul 29 2022, 4:57 PM
Harbormaster completed remote builds in B46683: Diff 108716.Jul 29 2022, 4:57 PM
jhb added a parent revision: D35990: wg: Use plain loads/stores for keepalive management..Jul 29 2022, 4:57 PM
jhb added a child revision: D35992: wg_noise: Drop atomics for kp_can_send..
emaste added a subscriber: emaste.Aug 2 2022, 4:30 PM

There is one unlocked read of the pointer for which the atomic load doesn't do anything (in terms of limiting races).

Does it make sense for this case to have a comment (i.e., explaining why the race is not important)?

jhb added inline comments.Aug 2 2022, 6:28 PM
sys/dev/wg/wg_noise.c
730

This is the unlocked read and it's just an optimization to avoid the lock below (and not an uncommon pattern, so I think it is obvious in context)

emaste added inline comments.Aug 2 2022, 6:52 PM
sys/dev/wg/wg_noise.c
730

Fair enough, I did not look at each case to find the unlocked read and when pointed out in context it's clear enough to me. [That said I don't think a comment wouldn't be out of place]

jhb added inline comments.Oct 6 2022, 10:32 PM
sys/dev/wg/wg_noise.c
589

Possibly these two should remain as atomics as they are used to provide READ_ONCE semantics.

706

Another READ_ONCE

865

Another READ_ONCE

894

Another READ_ONCE

jhb retitled this revision from wg_noise: Remove atomics for kp pointers. to wg_noise: Remove some atomics for kp pointers..Oct 7 2022, 7:35 PM
jhb edited the summary of this revision. (Show Details)
jhb updated this revision to Diff 111565.Oct 7 2022, 7:36 PM
jhb edited the summary of this revision. (Show Details)
  • Keep atomics for READ_ONCE cases.
Harbormaster completed remote builds in B47741: Diff 111565.Oct 7 2022, 7:36 PM
jhb retitled this revision from wg_noise: Remove some atomics for kp pointers. to wg: Remove some atomics for kp pointers..Oct 7 2022, 9:55 PM
jhb added a child revision: D36910: wg: Use atomic(9) instead of concurrency-kit atomics..Oct 7 2022, 10:03 PM
jhb removed a child revision: D35992: wg_noise: Drop atomics for kp_can_send..
jhb updated this revision to Diff 112160.Oct 24 2022, 7:08 PM
  • Rebase post atomic(9).
Harbormaster completed remote builds in B47978: Diff 112160.Oct 24 2022, 7:08 PM
jhb removed a child revision: D36910: wg: Use atomic(9) instead of concurrency-kit atomics..Oct 24 2022, 7:11 PM
jhb removed a parent revision: D35990: wg: Use plain loads/stores for keepalive management..

Revision Contents
Changeset List

PathSize
sys/
dev/
wg/
52 lines

Diff 108716

View Options

sys/dev/wg/wg_noise.c

Loading...