[PATCH 1/1] libcamera: add method to set thread affinity

[Kieran Bingham]

Quoting Harvey Yang (2024-10-11 18:00:48)
> From: Han-Lin Chen <hanlinchen at chromium.org>
> 
> Add method to set thread affinity to Thread class.
> 
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> ---
>  include/libcamera/base/thread.h |  6 ++++++
>  src/libcamera/base/thread.cpp   | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h
> index 4f33de63d..c74fe102b 100644
> --- a/include/libcamera/base/thread.h
> +++ b/include/libcamera/base/thread.h
> @@ -35,6 +35,8 @@ public:
>         void exit(int code = 0);
>         bool wait(utils::duration duration = utils::duration::max());
>  
> +       void setThreadAffinity(const std::vector<int> &cpus);
> +
>         bool isRunning();
>  
>         Signal<> finished;
> @@ -54,6 +56,8 @@ private:
>         void startThread();
>         void finishThread();
>  
> +       void setThreadAffinityInternal();
> +
>         void postMessage(std::unique_ptr<Message> msg, Object *receiver);
>         void removeMessages(Object *receiver);
>  
> @@ -67,6 +71,8 @@ private:
>  
>         std::thread thread_;
>         ThreadData *data_;
> +
> +       std::vector<int> cpus_;
>  };
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
> index 8735670b8..2417043a3 100644
> --- a/src/libcamera/base/thread.cpp
> +++ b/src/libcamera/base/thread.cpp
> @@ -254,6 +254,8 @@ void Thread::start()
>         data_->exit_.store(false, std::memory_order_relaxed);
>  
>         thread_ = std::thread(&Thread::startThread, this);
> +
> +       setThreadAffinityInternal();
>  }
>  
>  void Thread::startThread()
> @@ -410,6 +412,35 @@ bool Thread::wait(utils::duration duration)
>         return hasFinished;
>  }
>  
> +/**
> + * \brief Set the CPU affinity mask of the thread
> + * \param[in] cpus The list of CPU indices that the thread is set affinity to
> + */
> +void Thread::setThreadAffinity(const std::vector<int> &cpus)
> +{
> +       cpus_ = cpus;
> +

Should there be any checks here to verify the CPU list is valid? What
happens if I try to call Thread::setThreadAffinity(-1, 1000000) ?

Perhaps it should also ensure cpus.size() is never bigger than
CPU_SETSIZE?

The asynchronous nature here means we won't know if the call to
pthread_setaffinity_np() is successful until the thread is started, but
is there anything we can do to validate the input as this is extending
the Thread API? 

I'm not sure if there is anything worthwhile yet, but is there anything
around this that should then be added to the unit tests?


I know all of the above is really unlikely to be called incorrectly in
the code you'll add to use these - but we should treat libcamera-base as
if it were an entirely separate helper library so we shouldn't skimp on
layer checks and validation.


> +       MutexLocker locker(data_->mutex_);
> +       if (data_->running_)

I'ts equivalent, so it's fine like the above, but there's also the
helper which could make this:

	if (isRunning())

But both are fine with me so either way.

--
Kieran

> +               setThreadAffinityInternal();
> +}
> +
> +void Thread::setThreadAffinityInternal()
> +{
> +       if (cpus_.empty())
> +               return;
> +
> +       auto handle = thread_.native_handle();
> +
> +       cpu_set_t cpuset;
> +       CPU_ZERO(&cpuset);
> +
> +       for (auto cpu : cpus_)
> +               CPU_SET(cpu, &cpuset);
> +
> +       pthread_setaffinity_np(handle, sizeof(cpu_set_t), &cpuset);
> +}
> +
>  /**
>   * \brief Check if the thread is running
>   *
> -- 
> 2.47.0.rc1.288.g06298d1525-goog
>