[PATCH v7 1/1] libcamera: add method to set thread affinity
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Nov 29 18:26:11 CET 2024
Quoting Kieran Bingham (2024-11-20 11:22:34)
> Quoting Harvey Yang (2024-10-29 08:57:55)
> > 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>
>
> Looks good to me.
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
I have merged this patch based on the tests succeeding at :
- https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1319468
Since then the following pipelines have successfully also passed on top
of this patch:
- https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1319906
- https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1319907
However, I am now seeing failures at
- https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1320174
with the following jobs:
- https://gitlab.freedesktop.org/camera/libcamera/-/jobs/67448427
- https://gitlab.freedesktop.org/camera/libcamera/-/jobs/67445198
63/78 libcamera / threads FAIL 1.19s exit status 1
>>> MALLOC_PERTURB_=148 LD_LIBRARY_PATH=/builds/camera/libcamera/build/src/libcamera/base:/builds/camera/libcamera/build/src/libcamera /builds/camera/libcamera/build/test/threads
――――――――――――――――――――――――――――――――――――― ✀ ―――――――――――――――――――――――――――――――――――――
stderr:
AddressSanitizer:DEADLYSIGNAL
=================================================================
==909==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000002d0 (pc 0x7f5f6eab1670 bp 0x7f5f6cbfecf0 sp 0x7f5f6cbfecc8 T25)
==909==The signal is caused by a READ memory access.
==909==Hint: address points to the zero page.
#0 0x7f5f6eab1670 in pthread_setaffinity_np (/lib/x86_64-linux-gnu/libc.so.6+0x8f670)
#1 0x7f5f6eed73df in libcamera::Thread::setThreadAffinityInternal() ../src/libcamera/base/thread.cpp:457
#2 0x7f5f6eed62be in libcamera::Thread::startThread() ../src/libcamera/base/thread.cpp:287
#3 0x7f5f6eee0e96 in void std::__invoke_impl<void, void (libcamera::Thread::*)(), libcamera::Thread*>(std::__invoke_memfun_deref, void (libcamera::Thread::*&&)(), libcamera::Thread*&&) /usr/include/c++/12/bits/invoke.h:74
#4 0x7f5f6eee0d02 in std::__invoke_result<void (libcamera::Thread::*)(), libcamera::Thread*>::type std::__invoke<void (libcamera::Thread::*)(), libcamera::Thread*>(void (libcamera::Thread::*&&)(), libcamera::Thread*&&) /usr/include/c++/12/bits/invoke.h:96
#5 0x7f5f6eee0c72 in void std::thread::_Invoker<std::tuple<void (libcamera::Thread::*)(), libcamera::Thread*> >::_M_invoke<0ul, 1ul>(std::_Index_tuple<0ul, 1ul>) /usr/include/c++/12/bits/std_thread.h:252
#6 0x7f5f6eee0c2b in std::thread::_Invoker<std::tuple<void (libcamera::Thread::*)(), libcamera::Thread*> >::operator()() /usr/include/c++/12/bits/std_thread.h:259
#7 0x7f5f6eee0c0f in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (libcamera::Thread::*)(), libcamera::Thread*> > >::_M_run() /usr/include/c++/12/bits/std_thread.h:210
#8 0x7f5f6ecf74a2 (/lib/x86_64-linux-gnu/libstdc++.so.6+0xd44a2)
#9 0x7f5f6eaab1c3 (/lib/x86_64-linux-gnu/libc.so.6+0x891c3)
#10 0x7f5f6eb2b85b (/lib/x86_64-linux-gnu/libc.so.6+0x10985b)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib/x86_64-linux-gnu/libc.so.6+0x8f670) in pthread_setaffinity_np
Thread T25 created by T0 here:
#0 0x7f5f6ef5c726 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:207
#1 0x7f5f6ecf7578 in std::thread::_M_start_thread(std::unique_ptr<std::thread::_State, std::default_delete<std::thread::_State> >, void (*)()) (/lib/x86_64-linux-gnu/libstdc++.so.6+0xd4578)
#2 0x7f5f6eed5ddb in libcamera::Thread::start() ../src/libcamera/base/thread.cpp:259
#3 0x55dea5dbf519 in ThreadTest::run() ../test/threads.cpp:196
#4 0x55dea5dc5bf4 in Test::execute() ../test/libtest/test.cpp:33
#5 0x55dea5dbd54f in main ../test/threads.cpp:216
#6 0x7f5f6ea49249 (/lib/x86_64-linux-gnu/libc.so.6+0x27249)
==909==ABORTING
I'm not yet sure what to do here, it might be that we need to revert the
ThreadAffinity patch.
Harvey, could you see if you can replicate this issue - or spot any
potential issues here please?
--
Kieran
>
> > ---
> > include/libcamera/base/thread.h | 5 ++++
> > src/libcamera/base/thread.cpp | 47 +++++++++++++++++++++++++++++++++
> > test/threads.cpp | 40 ++++++++++++++++++++++++++++
> > 3 files changed, 92 insertions(+)
> >
> > diff --git a/include/libcamera/base/thread.h b/include/libcamera/base/thread.h
> > index 4f33de63d..3209d4f7c 100644
> > --- a/include/libcamera/base/thread.h
> > +++ b/include/libcamera/base/thread.h
> > @@ -15,6 +15,7 @@
> >
> > #include <libcamera/base/message.h>
> > #include <libcamera/base/signal.h>
> > +#include <libcamera/base/span.h>
> > #include <libcamera/base/utils.h>
> >
> > namespace libcamera {
> > @@ -35,6 +36,8 @@ public:
> > void exit(int code = 0);
> > bool wait(utils::duration duration = utils::duration::max());
> >
> > + int setThreadAffinity(const Span<const unsigned int> &cpus);
> > +
> > bool isRunning();
> >
> > Signal<> finished;
> > @@ -54,6 +57,8 @@ private:
> > void startThread();
> > void finishThread();
> >
> > + void setThreadAffinityInternal();
> > +
> > void postMessage(std::unique_ptr<Message> msg, Object *receiver);
> > void removeMessages(Object *receiver);
> >
> > diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
> > index 8735670b8..f6322fe31 100644
> > --- a/src/libcamera/base/thread.cpp
> > +++ b/src/libcamera/base/thread.cpp
> > @@ -9,6 +9,7 @@
> >
> > #include <atomic>
> > #include <list>
> > +#include <optional>
> > #include <sys/syscall.h>
> > #include <sys/types.h>
> > #include <unistd.h>
> > @@ -128,6 +129,8 @@ private:
> > int exitCode_;
> >
> > MessageQueue messages_;
> > +
> > + std::optional<cpu_set_t> cpuset_;
> > };
> >
> > /**
> > @@ -281,6 +284,8 @@ void Thread::startThread()
> > data_->tid_ = syscall(SYS_gettid);
> > currentThreadData = data_;
> >
> > + setThreadAffinityInternal();
> > +
> > run();
> > }
> >
> > @@ -410,6 +415,48 @@ 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
> > + *
> > + * The CPU indices should be within [0, std::thread::hardware_concurrency()).
> > + * If any index is invalid, this function won't modify the thread affinity and
> > + * will return an error.
> > + *
> > + * \return 0 if all indices are valid, -EINVAL otherwise
> > + */
> > +int Thread::setThreadAffinity(const Span<const unsigned int> &cpus)
> > +{
> > + const unsigned int numCpus = std::thread::hardware_concurrency();
> > +
> > + MutexLocker locker(data_->mutex_);
> > + data_->cpuset_ = cpu_set_t();
> > + CPU_ZERO(&data_->cpuset_.value());
> > +
> > + for (const unsigned int &cpu : cpus) {
> > + if (cpu >= numCpus) {
> > + LOG(Thread, Error) << "Invalid CPU " << cpu << "for thread affinity";
> > + return -EINVAL;
> > + }
> > +
> > + CPU_SET(cpu, &data_->cpuset_.value());
> > + }
> > +
> > + if (data_->running_)
> > + setThreadAffinityInternal();
> > +
> > + return 0;
> > +}
> > +
> > +void Thread::setThreadAffinityInternal()
> > +{
> > + if (!data_->cpuset_)
> > + return;
> > +
> > + const cpu_set_t &cpuset = data_->cpuset_.value();
> > + pthread_setaffinity_np(thread_.native_handle(), sizeof(cpuset), &cpuset);
> > +}
> > +
> > /**
> > * \brief Check if the thread is running
> > *
> > diff --git a/test/threads.cpp b/test/threads.cpp
> > index ceb4fa0f2..8d6ee1510 100644
> > --- a/test/threads.cpp
> > +++ b/test/threads.cpp
> > @@ -9,9 +9,11 @@
> > #include <iostream>
> > #include <memory>
> > #include <pthread.h>
> > +#include <sched.h>
> > #include <thread>
> > #include <time.h>
> >
> > +#include <libcamera/base/object.h>
> > #include <libcamera/base/thread.h>
> >
> > #include "test.h"
> > @@ -66,6 +68,27 @@ private:
> > bool &cancelled_;
> > };
> >
> > +class CpuSetTester : public Object
> > +{
> > +public:
> > + CpuSetTester(unsigned int cpuset)
> > + : cpuset_(cpuset) {}
> > +
> > + bool testCpuSet()
> > + {
> > + int ret = sched_getcpu();
> > + if (static_cast<int>(cpuset_) != ret) {
> > + cout << "Invalid cpuset: " << ret << ", expecting: " << cpuset_ << endl;
> > + return false;
> > + }
> > +
> > + return true;
> > + }
> > +
> > +private:
> > + const unsigned int cpuset_;
> > +};
> > +
> > class ThreadTest : public Test
> > {
> > protected:
> > @@ -165,6 +188,23 @@ protected:
> > return TestFail;
> > }
> >
> > + const unsigned int numCpus = std::thread::hardware_concurrency();
> > + for (unsigned int i = 0; i < numCpus; ++i) {
> > + thread = std::make_unique<Thread>();
> > + const std::array<const unsigned int, 1> cpus{ i };
> > + thread->setThreadAffinity(cpus);
> > + thread->start();
> > +
> > + CpuSetTester tester(i);
> > + tester.moveToThread(thread.get());
> > +
> > + if (!tester.invokeMethod(&CpuSetTester::testCpuSet, ConnectionTypeBlocking))
> > + return TestFail;
> > +
> > + thread->exit(0);
> > + thread->wait();
> > + }
> > +
> > return TestPass;
> > }
> >
> > --
> > 2.47.0.163.g1226f6d8fa-goog
> >
More information about the libcamera-devel
mailing list