[{"id":33162,"web_url":"https://patchwork.libcamera.org/comment/33162/","msgid":"<20250124172908.GE13689@pendragon.ideasonboard.com>","date":"2025-01-24T17:29:08","subject":"Re: [RFC PATCH v1 4/7] libcamera: base: log: Make\n\t`LogCategory::severity_` atomic","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Barnabás,\n\nThank you for the patch.\n\nOn Tue, Jan 21, 2025 at 06:56:02PM +0000, Barnabás Pőcze wrote:\n> The severity of a log category may be changed from a different thread,\n> so it is important to ensure that the reads and writes happen atomically.\n> Using `std::memory_order_relaxed` should not introduce any synchronization\n> overhead, it should only guarantee that the operation itself is atomic.\n> \n> Secondly, inline `LogCategory::setSeverity()`, as it is merely an\n> assignment, so going through a DSO call is a big pessimization.\n> `LogCategory` is not part of the public API, so this change has\n> no external effects.\n> \n> Thirdly, assert that the atomic variable is lock free so as to ensure\n> it won't silently fall back to libatomic (or similar) on any platform.\n> If this assertion fails, this needs to be revisited.\n> \n> Signed-off-by: Barnabás Pőcze \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> include/libcamera/base/log.h | 9 ++++++---\n> src/libcamera/base/log.cpp | 5 +----\n> 2 files changed, 7 insertions(+), 7 deletions(-)\n> \n> diff --git a/include/libcamera/base/log.h b/include/libcamera/base/log.h\n> index d73aa7913..89f707e54 100644\n> --- a/include/libcamera/base/log.h\n> +++ b/include/libcamera/base/log.h\n> @@ -7,6 +7,7 @@\n> \n> #pragma once\n> \n> +#include \n> #include \n> \n> #include \n> @@ -31,8 +32,8 @@ public:\n> \tstatic LogCategory *create(const char *name);\n> \n> \tconst std::string &name() const { return name_; }\n> -\tLogSeverity severity() const { return severity_; }\n> -\tvoid setSeverity(LogSeverity severity);\n> +\tLogSeverity severity() const { return severity_.load(std::memory_order_relaxed); }\n> +\tvoid setSeverity(LogSeverity severity) { severity_.store(severity, std::memory_order_relaxed); }\n> \n> \tstatic const LogCategory &defaultCategory();\n> \n> @@ -40,7 +41,9 @@ private:\n> \texplicit LogCategory(const char *name);\n> \n> \tconst std::string name_;\n> -\tLogSeverity severity_;\n> +\n> +\tstd::atomic severity_;\n> +\tstatic_assert(decltype(severity_)::is_always_lock_free);\n> };\n> \n> #define LOG_DECLARE_CATEGORY(name)\t\t\t\t\t\\\n> diff --git a/src/libcamera/base/log.cpp b/src/libcamera/base/log.cpp\n> index 1513e41fb..795260b0a 100644\n> --- a/src/libcamera/base/log.cpp\n> +++ b/src/libcamera/base/log.cpp\n> @@ -825,15 +825,12 @@ LogCategory::LogCategory(const char *name)\n> */\n> \n> /**\n> + * \\fn LogCategory::setSeverity(LogSeverity severity)\n> * \\brief Set the severity of the log category\n> *\n> * Messages of severity higher than or equal to the severity of the log category\n> * are printed, other messages are discarded.\n> */\n> -void LogCategory::setSeverity(LogSeverity severity)\n> -{\n> -\tseverity_ = severity;\n> -}\n> \n> /**\n> * \\brief Retrieve the default log category","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CD484C322E\n\tfor ;\n\tFri, 24 Jan 2025 17:29:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D93C76855D;\n\tFri, 24 Jan 2025 18:29:19 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 90A2868556\n\tfor ;\n\tFri, 24 Jan 2025 18:29:18 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F1FBC4AD;\n\tFri, 24 Jan 2025 18:28:13 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"aMvzf0PR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737739694;\n\tbh=ezhqNQj+ng8SrCBSIIaPG0TQE0DpufJVvIqkLdHj6lk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aMvzf0PRMtuNVPM+8xXhJsSK/JwSKgjePdoTbvsh1eqPPzqTRyp+OO529fTqb3Wqu\n\t/ezNQwkbHL3DqzJbm9icqSWzUUYSKSuKsMS+ZTj1+C0coWKQqjNijCW1ijD1qf79Ql\n\tTTyoeQq6T3fYV7jvFcEiqOCMpEZ/JHz9qfj6KF9E=","Date":"Fri, 24 Jan 2025 19:29:08 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [RFC PATCH v1 4/7] libcamera: base: log: Make\n\t`LogCategory::severity_` atomic","Message-ID":"<20250124172908.GE13689@pendragon.ideasonboard.com>","References":"<20250121185554.301901-1-pobrn@protonmail.com>\n\t<20250121185554.301901-2-pobrn@protonmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250121185554.301901-2-pobrn@protonmail.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]