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

Jump to bottom

datapath/linux: document methods that must be called with mutex held #12472

Merged
merged 1 commit into from Jul 9, 2020
Merged

datapath/linux: document methods that must be called with mutex held #12472

merged 1 commit into from Jul 9, 2020

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Jul 9, 2020

Follow-up for #12461 per review comment
#12461 (comment)

Suggested-by: Tam Mach sayboras@yahoo.com

/cc @sayboras

@tklauser
datapath/linux: document methods that must be called with mutex held
b4bba49 
Follow-up for #12461 per review comment
#12461 (comment)

Suggested-by: Tam Mach <sayboras@yahoo.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser added the release-note/misc This PR makes changes that have no direct user impact. label Jul 9, 2020
@tklauser tklauser requested a review from a team July 9, 2020 13:04
sayboras
sayboras approved these changes Jul 9, 2020
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad that my comment is making some sense 💯

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 36.958% when pulling b4bba49 on pr/tklauser/neighbynode-mutex-comment into 9fe5b33 on master.

@pchaigno
Copy link
Member

pchaigno commented Jul 9, 2020

Only comment changes, so I'm marking this as ready to merge.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 9, 2020
@aanm aanm merged commit 9db20dc into master Jul 9, 2020
@aanm aanm deleted the pr/tklauser/neighbynode-mutex-comment branch July 9, 2020 15:21
joestringer
joestringer reviewed Jul 9, 2020
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW we often name functions like this fooLocked so it's clear from (1) the code and (2) gops stack traces that this function expected to handle a lock. This can be useful to track down lock misuse in live environments.

@joestringer joestringer mentioned this pull request Oct 12, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer left review comments

@pchaigno pchaigno pchaigno approved these changes

@sayboras sayboras sayboras approved these changes

Assignees
No one assigned
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@tklauser @coveralls @pchaigno @joestringer @sayboras @aanm