[libcamera-devel] [PATCH 1/2] libcamera: thread: Add a method to return the ID of the current thread

[Laurent Pinchart]

Hi Kieran,

On Mon, Jan 20, 2020 at 10:11:58AM +0000, Kieran Bingham wrote:
> On 18/01/2020 21:33, Laurent Pinchart wrote:
> > The current thread ID is useful when logging message to debug
> > concurrency issues. Add a method to retrieve it.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/libcamera/include/thread.h |  2 ++
> >  src/libcamera/thread.cpp       | 20 ++++++++++++++++++++
> >  2 files changed, 22 insertions(+)
> > 
> > diff --git a/src/libcamera/include/thread.h b/src/libcamera/include/thread.h
> > index 37edd4f5138b..819a9879ac56 100644
> > --- a/src/libcamera/include/thread.h
> > +++ b/src/libcamera/include/thread.h
> > @@ -9,6 +9,7 @@
> >  
> >  #include <memory>
> >  #include <mutex>
> > +#include <sys/types.h>
> >  #include <thread>
> >  
> >  #include <libcamera/signal.h>
> > @@ -39,6 +40,7 @@ public:
> >  	Signal<Thread *> finished;
> >  
> >  	static Thread *current();
> > +	static pid_t currentId();
> >  
> >  	EventDispatcher *eventDispatcher();
> >  	void setEventDispatcher(std::unique_ptr<EventDispatcher> dispatcher);
> > diff --git a/src/libcamera/thread.cpp b/src/libcamera/thread.cpp
> > index 18ebd16a7e2f..fe32cd677596 100644
> > --- a/src/libcamera/thread.cpp
> > +++ b/src/libcamera/thread.cpp
> > @@ -9,6 +9,9 @@
> >  
> >  #include <atomic>
> >  #include <list>
> > +#include <unistd.h>
> > +#include <sys/syscall.h>
> > +#include <sys/types.h>
> >  
> >  #include <libcamera/event_dispatcher.h>
> >  
> > @@ -62,6 +65,7 @@ private:
> >  
> >  	Thread *thread_;
> >  	bool running_;
> > +	pid_t tid_;
> >  
> >  	Mutex mutex_;
> >  
> > @@ -108,6 +112,7 @@ ThreadData *ThreadData::current()
> >  	 * started, set it here.
> >  	 */
> >  	ThreadData *data = mainThread.data_;
> > +	data->tid_ = syscall(SYS_gettid);
> >  	currentThreadData = data;
> >  	return data;
> >  }
> > @@ -189,6 +194,7 @@ void Thread::startThread()
> >  	 */
> >  	thread_local ThreadCleaner cleaner(this, &Thread::finishThread);
> >  
> > +	data_->tid_ = syscall(SYS_gettid);
> 
> We will now have direct syscalls from V4L2Device, V4L2VideoDevice, and
> Thread.
> 
> Not that we particularly want another-adaptation-layer, but should these
> be moved to a single layer (or even just utils::?), as effectively they
> are c-library emulations?

For the openat() syscall, we may want to do so, as it may need to be
wrapped with additional logic (for instance to handle 32/64-bit file
access, if needed). For gettid here, I'm not sure, as there's no
gettid() in C libraries. And now that I've written this, I realize that
glibc has added a gettid() wrapper in v2.30 (released on August the 1st
2019). We could thus add a wrapper too, but more as a convenience than
anything else, as it's just one syscall without any glue around it.

> Anyway, if we chose to do so, that can be a standalone patch on top.
> 
> This is certainly a good idea to help threaded debug.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> >  	currentThreadData = data_;
> >  
> >  	run();
> > @@ -308,6 +314,20 @@ Thread *Thread::current()
> >  	return data->thread_;
> >  }
> >  
> > +/**
> > + * \brief Retrieve the ID of the current thread
> > + *
> > + * The thread ID corresponds to the Linux thread ID (TID) as returned by the
> > + * gettid system call.
> > + *
> > + * \return The ID of the current thread
> > + */
> > +pid_t Thread::currentId()
> > +{
> > +	ThreadData *data = ThreadData::current();
> > +	return data->tid_;
> > +}
> > +
> >  /**
> >   * \brief Set the event dispatcher
> >   * \param[in] dispatcher Pointer to the event dispatcher

-- 
Regards,

Laurent Pinchart