Skip to content

Commit

Permalink
FW/logging: use atomics for the message count and data size
Browse files Browse the repository at this point in the history
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.

Helps opendcdiag#153.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
  • Loading branch information
thiagomacieira committed Sep 10, 2022
1 parent ac0d867 commit a3b361b
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 12 deletions.
16 changes: 6 additions & 10 deletions framework/logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -729,12 +729,10 @@ static void log_message_preformatted(int thread_num, std::string_view msg)
if (msg[0] == 'E')
logging_mark_thread_failed(thread_num);

int &messages_logged = cpu_data_for_thread(thread_num)->messages_logged;
if (messages_logged >= sApp->max_messages_per_thread)
std::atomic<int> &messages_logged = cpu_data_for_thread(thread_num)->messages_logged;
if (messages_logged.fetch_add(1, std::memory_order_relaxed) >= sApp->max_messages_per_thread)
return;

messages_logged++;

if (msg[msg.size() - 1] == '\n')
msg.remove_suffix(1); // remove trailing newline

Expand Down Expand Up @@ -1306,14 +1304,12 @@ void log_data(const char *message, const void *data, size_t size)
if (current_output_format() == SandstoneApplication::OutputFormat::no_output)
return; // short-circuit

int &messages_logged = cpu_data_for_thread(thread_num)->messages_logged;
size_t &data_bytes_logged = cpu_data_for_thread(thread_num)->data_bytes_logged;
if ((messages_logged >= sApp->max_messages_per_thread) ||
(data_bytes_logged + size > sApp->max_logdata_per_thread))
std::atomic<int> &messages_logged = cpu_data_for_thread(thread_num)->messages_logged;
std::atomic<size_t> &data_bytes_logged = cpu_data_for_thread(thread_num)->data_bytes_logged;
if (messages_logged.fetch_add(1, std::memory_order_relaxed) >= sApp->max_messages_per_thread ||
(data_bytes_logged.fetch_add(size, std::memory_order_relaxed) > sApp->max_logdata_per_thread))
return;

messages_logged++;
data_bytes_logged += size;

log_data_common(message, static_cast<const uint8_t *>(data), size, false);
}
Expand Down
4 changes: 2 additions & 2 deletions framework/sandstone_p.h
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,10 @@ struct alignas(64) per_thread_data
std::atomic<ThreadState> thread_state;

/* Records number of messages logged per thread of each test */
int messages_logged;
std::atomic<int> messages_logged;

/* Records the number of bytes log_data'ed per thread */
size_t data_bytes_logged;
std::atomic<size_t> data_bytes_logged;

/* Number of iterations of the inner loop (aka #times test_time_condition called) */
uint64_t inner_loop_count;
Expand Down

0 comments on commit a3b361b

Please sign in to comment.