[PATCH v2] Thread: Fix setThreadAffinity race condition in start

[Laurent Pinchart]

Hi Harvey,

Thank you for the patch.

On Tue, Jan 07, 2025 at 02:41:37PM +0000, Harvey Yang wrote:
> Previously we call Thread::setThreadAffinityInternal in
> Thread::startThread. The purpose was to avoid the main workload being
> run on incorrect CPUs. This leads to a race condition of setting
> `Thread::thread_` in `Thread::start()` and accessing
> `Thread::setThreadAffinityInternal` though.
> 
> This patch moves the call after the construction of std::thread to avoid
> the race condition. The downside is that the first tasks, if any, upon
> starting a thread might be run on incorrect CPUs.
> 
> Fixes: 4d9db06d6690 ("libcamera: add method to set thread affinity")
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>

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

> ---
>  src/libcamera/base/thread.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
> index f6322fe3108b..319bfda9ae98 100644
> --- a/src/libcamera/base/thread.cpp
> +++ b/src/libcamera/base/thread.cpp
> @@ -257,6 +257,8 @@ void Thread::start()
>  	data_->exit_.store(false, std::memory_order_relaxed);
>  
>  	thread_ = std::thread(&Thread::startThread, this);
> +
> +	setThreadAffinityInternal();
>  }
>  
>  void Thread::startThread()
> @@ -284,8 +286,6 @@ void Thread::startThread()
>  	data_->tid_ = syscall(SYS_gettid);
>  	currentThreadData = data_;
>  
> -	setThreadAffinityInternal();
> -
>  	run();
>  }
>  

-- 
Regards,

Laurent Pinchart