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

FW/logging: refactor log_{platform_,}message #154

Conversation

thiagomacieira
Copy link
Contributor

@thiagomacieira thiagomacieira commented Sep 10, 2022

This merges the two functions for the most part and makes them use a single writev() call to write the content.

This was originally done to fix #153, but that fails because the Linux kernel does not guarantee atomicity of write() calls and I wouldn't care to make a guess what other OSes do. If we want to have log_platform_message() be thread-safe, we'll need to insert a mutex.

Signed-off-by: Thiago Macieira thiago.macieira@intel.com

@thiagomacieira
Copy link
Contributor Author

Didn't work.

@thiagomacieira thiagomacieira deleted the fix-log-platform-message-race branch February 22, 2023 17:56
@thiagomacieira thiagomacieira restored the fix-log-platform-message-race branch February 22, 2023 17:57
@thiagomacieira thiagomacieira changed the title Fix log_platform_message race FW/logging: refactor log_{platform_,}message Feb 22, 2023
Our logging FILE* are opened in unbuffered mode.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
log_data() is probably only used by tests, so there's no race problem
there. But log_message() may be used on the main thread's messaging,
which could cause a race condition.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
This merges the two functions for the most part and makes them use a
single writev() call to write the content.

This was originally done to fix opendcdiag#153, but that fails because the Linux
kernel does not guarantee atomicity of write() calls and I wouldn't care
to make a guess what other OSes do. If we want to have
log_platform_message() be thread-safe, we'll need to insert a mutex.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
@thiagomacieira thiagomacieira marked this pull request as ready for review February 22, 2023 18:33
@thiagomacieira
Copy link
Contributor Author

This is old code.

@thiagomacieira thiagomacieira deleted the fix-log-platform-message-race branch May 8, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible race condition somewhere in log_platform_message
2 participants