[RFC PATCH v1 4/7] libcamera: base: log: Make `LogCategory::severity_` atomic

[Laurent Pinchart]

Hi Barnabás,

Thank you for the patch.

On Tue, Jan 21, 2025 at 06:56:02PM +0000, Barnabás Pőcze wrote:
> The severity of a log category may be changed from a different thread,
> so it is important to ensure that the reads and writes happen atomically.
> Using `std::memory_order_relaxed` should not introduce any synchronization
> overhead, it should only guarantee that the operation itself is atomic.
> 
> Secondly, inline `LogCategory::setSeverity()`, as it is merely an
> assignment, so going through a DSO call is a big pessimization.
> `LogCategory` is not part of the public API, so this change has
> no external effects.
> 
> Thirdly, assert that the atomic variable is lock free so as to ensure
> it won't silently fall back to libatomic (or similar) on any platform.
> If this assertion fails, this needs to be revisited.
> 
> Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  include/libcamera/base/log.h | 9 ++++++---
>  src/libcamera/base/log.cpp   | 5 +----
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h
> index d73aa7913..89f707e54 100644
> --- a/include/libcamera/base/log.h
> +++ b/include/libcamera/base/log.h
> @@ -7,6 +7,7 @@
>  
>  #pragma once
>  
> +#include <atomic>
>  #include <sstream>
>  
>  #include <libcamera/base/private.h>
> @@ -31,8 +32,8 @@ public:
>  	static LogCategory *create(const char *name);
>  
>  	const std::string &name() const { return name_; }
> -	LogSeverity severity() const { return severity_; }
> -	void setSeverity(LogSeverity severity);
> +	LogSeverity severity() const { return severity_.load(std::memory_order_relaxed); }
> +	void setSeverity(LogSeverity severity) { severity_.store(severity, std::memory_order_relaxed); }
>  
>  	static const LogCategory &defaultCategory();
>  
> @@ -40,7 +41,9 @@ private:
>  	explicit LogCategory(const char *name);
>  
>  	const std::string name_;
> -	LogSeverity severity_;
> +
> +	std::atomic<LogSeverity> severity_;
> +	static_assert(decltype(severity_)::is_always_lock_free);
>  };
>  
>  #define LOG_DECLARE_CATEGORY(name)					\
> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp
> index 1513e41fb..795260b0a 100644
> --- a/src/libcamera/base/log.cpp
> +++ b/src/libcamera/base/log.cpp
> @@ -825,15 +825,12 @@ LogCategory::LogCategory(const char *name)
>   */
>  
>  /**
> + * \fn LogCategory::setSeverity(LogSeverity severity)
>   * \brief Set the severity of the log category
>   *
>   * Messages of severity higher than or equal to the severity of the log category
>   * are printed, other messages are discarded.
>   */
> -void LogCategory::setSeverity(LogSeverity severity)
> -{
> -	severity_ = severity;
> -}
>  
>  /**
>   * \brief Retrieve the default log category

-- 
Regards,

Laurent Pinchart