Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

common/perf_counters: fix race condition with atomic variables #14227

Merged
merged 1 commit into from Apr 8, 2017

Conversation

Projects
None yet
3 participants
@ivancich
Copy link
Member

ivancich commented Mar 29, 2017

Please check my logic for this apparent fix. This technique of using two matched atomic variables as a guard against race conditions is new to me.

Fix potential race condition. With the old code it would be possible
to get incorrect pair if the following sequence occurred:

    ==inc==          ==read_avg==
    avgcount.inc()
                    avgcount.read()
A)                  u64.read()
    u64.add()
B)                  u64.read()
    avgcount2.inc()
                    avgcount2.read()

Depending on whether u64.read() is called at A) or B) we'd get the new
count with either the old value or new value. By flipping the order of
the avgcount & avgcount2 reads we're guaranteed to only get matched
values.

Signed-off-by: J. Eric Ivancich ivancich@redhat.com

common/perf_counters: fix race condition with atomic variables
Fix potential race condition. With the old code it would be possible
to get incorrect pair if the following sequence occured:

    ==inc==          ==read_avg==
    avgcount.inc()
                    avgcount.read()
A)                  u64.read()
    u64.add()
B)                  u64.read()
    avgcount2.inc()
                    avgcount2.read()

Depending on whether u64.read() is called at A) or B) we'd get the new
count with either the old value or new value. By flipping the order of
the avgcount & avgcount2 reads we're guaranteed to only get matched
values.

Signed-off-by: J. Eric Ivancich <ivancich@redhat.com>

@ivancich ivancich requested review from liewegas and jdurgin Mar 29, 2017

@liewegas
Copy link
Member

liewegas left a comment

This fix looks right to me. @jdurgin ?

@jdurgin
Copy link
Member

jdurgin left a comment

Nice catch, and excellent commit message too!

@ivancich ivancich changed the title [DNM] common/perf_counters: fix race condition with atomic variables common/perf_counters: fix race condition with atomic variables Apr 1, 2017

@liewegas liewegas merged commit bcb17aa into ceph:master Apr 8, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.