[v2,1/2] eal: expose lcore pthread id
Checks
Commit Message
From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
Also expose the pthread id of each lcore, in
order to allow modification of pthread attributes,
for example use rte_thread_setname without executing
pthread_self() on the maybe already running lcore.
The rte_lcore_to_thread_id function is added to API.
Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
---
lib/eal/common/eal_common_lcore.c | 10 ++++++++++
lib/eal/include/rte_lcore.h | 14 ++++++++++++++
lib/eal/version.map | 1 +
3 files changed, 25 insertions(+)
Comments
On Fri, Oct 14, 2022 at 9:54 AM Markus Theil <markus.theil@tu-ilmenau.de> wrote:
>
> From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
>
> Also expose the pthread id of each lcore, in
> order to allow modification of pthread attributes,
> for example use rte_thread_setname without executing
> pthread_self() on the maybe already running lcore.
>
> The rte_lcore_to_thread_id function is added to API.
>
> Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
We are trying to abstract the use of pthread in DPDK API.
So I don't think this patch is going in the right direction.
Cc: Tyler.
On Thu, 20 Oct 2022 13:20:40 +0200
David Marchand <david.marchand@redhat.com> wrote:
> On Fri, Oct 14, 2022 at 9:54 AM Markus Theil <markus.theil@tu-ilmenau.de> wrote:
> >
> > From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> >
> > Also expose the pthread id of each lcore, in
> > order to allow modification of pthread attributes,
> > for example use rte_thread_setname without executing
> > pthread_self() on the maybe already running lcore.
> >
> > The rte_lcore_to_thread_id function is added to API.
> >
> > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
>
> We are trying to abstract the use of pthread in DPDK API.
> So I don't think this patch is going in the right direction.
Agree, exposing this will make Windows support harder
and who knows what next OS to come will need.
On Thu, 2022-10-20 at 08:36 -0700, Stephen Hemminger wrote:
> On Thu, 20 Oct 2022 13:20:40 +0200
> David Marchand <david.marchand@redhat.com> wrote:
>
> > On Fri, Oct 14, 2022 at 9:54 AM Markus Theil <markus.theil@tu-ilmenau.de>
> > wrote:
> > >
> > > From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> > >
> > > Also expose the pthread id of each lcore, in
> > > order to allow modification of pthread attributes,
> > > for example use rte_thread_setname without executing
> > > pthread_self() on the maybe already running lcore.
> > >
> > > The rte_lcore_to_thread_id function is added to API.
> > >
> > > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> >
> > We are trying to abstract the use of pthread in DPDK API.
> > So I don't think this patch is going in the right direction.
>
> Agree, exposing this will make Windows support harder
> and who knows what next OS to come will need.
Hi,
thanks for your feedback. I understand your concerns regarding abstraction and
portability.
Markus and I ultimately use the function in the patch to call
rte_thread_setname() (which takes the pthread id as an argument) to rename our
lcore workers from "lcore-worker-X" to something more meaningful in the scope
of our application. Having descriptive thread names makes debugging
significantly easier. For example, verifying CPU pinning worked as intended
with ps -T ..., or identifying threads in the Intel VTune profiler.
Would you consider something like
- int rte_lcore_setname(unsigned int lcore_id, const char *name)
- int rte_lcore_getname(unsigned int lcore_id, char *name, size_t len)
a more appropriate API? That would still allow us to set names from the main
lcore, but would not expose any pthread internals.
Thanks & regards
Michael
On Thu, Oct 20, 2022 at 08:36:05AM -0700, Stephen Hemminger wrote:
> On Thu, 20 Oct 2022 13:20:40 +0200
> David Marchand <david.marchand@redhat.com> wrote:
>
> > On Fri, Oct 14, 2022 at 9:54 AM Markus Theil <markus.theil@tu-ilmenau.de> wrote:
> > >
> > > From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> > >
> > > Also expose the pthread id of each lcore, in
> > > order to allow modification of pthread attributes,
> > > for example use rte_thread_setname without executing
> > > pthread_self() on the maybe already running lcore.
> > >
> > > The rte_lcore_to_thread_id function is added to API.
> > >
> > > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> >
> > We are trying to abstract the use of pthread in DPDK API.
> > So I don't think this patch is going in the right direction.
>
> Agree, exposing this will make Windows support harder
> and who knows what next OS to come will need.
+1
please don't expose platform details through eal you destroy
portability.
i'll take a closer look at this mail next week to see if i have anything
to offer.
On Thu, Oct 20, 2022 at 10:03:01PM +0200, Michael Pfeiffer wrote:
> On Thu, 2022-10-20 at 08:36 -0700, Stephen Hemminger wrote:
> > On Thu, 20 Oct 2022 13:20:40 +0200
> > David Marchand <david.marchand@redhat.com> wrote:
> >
> > > On Fri, Oct 14, 2022 at 9:54 AM Markus Theil <markus.theil@tu-ilmenau.de>
> > > wrote:
> > > >
> > > > From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> > > >
> > > > Also expose the pthread id of each lcore, in
> > > > order to allow modification of pthread attributes,
> > > > for example use rte_thread_setname without executing
> > > > pthread_self() on the maybe already running lcore.
> > > >
> > > > The rte_lcore_to_thread_id function is added to API.
> > > >
> > > > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> > >
> > > We are trying to abstract the use of pthread in DPDK API.
> > > So I don't think this patch is going in the right direction.
> >
> > Agree, exposing this will make Windows support harder
> > and who knows what next OS to come will need.
>
> Hi,
> thanks for your feedback. I understand your concerns regarding abstraction and
> portability.
>
> Markus and I ultimately use the function in the patch to call
> rte_thread_setname() (which takes the pthread id as an argument) to rename our
> lcore workers from "lcore-worker-X" to something more meaningful in the scope
> of our application. Having descriptive thread names makes debugging
> significantly easier. For example, verifying CPU pinning worked as intended
> with ps -T ..., or identifying threads in the Intel VTune profiler.
>
> Would you consider something like
> - int rte_lcore_setname(unsigned int lcore_id, const char *name)
> - int rte_lcore_getname(unsigned int lcore_id, char *name, size_t len)
> a more appropriate API? That would still allow us to set names from the main
> lcore, but would not expose any pthread internals.
if only setname, getname are needed for rte_thread_t i imagine it
shouldn't be too objectionable to add them to rte_thread.h
you can either wait until i have time to do it (could be a while) or you
can put a new patch + unit test together.
if testing the windows implementation part of the addition is a barrier
i can apply and run the supplied unit test, assist with review.
>
> Thanks & regards
> Michael
On Tue, 29 Nov 2022 14:04:45 -0800
Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:
> > Markus and I ultimately use the function in the patch to call
> > rte_thread_setname() (which takes the pthread id as an argument) to rename our
> > lcore workers from "lcore-worker-X" to something more meaningful in the scope
> > of our application. Having descriptive thread names makes debugging
> > significantly easier. For example, verifying CPU pinning worked as intended
> > with ps -T ..., or identifying threads in the Intel VTune profiler.
Why not have the worker threads rename themselves?
On Wed, 2023-07-05 at 19:57 -0700, Stephen Hemminger wrote:
> On Tue, 29 Nov 2022 14:04:45 -0800
> Tyler Retzlaff <roretzla@linux.microsoft.com> wrote:
>
> > > Markus and I ultimately use the function in the patch to call
> > > rte_thread_setname() (which takes the pthread id as an argument) to
> > > rename our
> > > lcore workers from "lcore-worker-X" to something more meaningful in the
> > > scope
> > > of our application. Having descriptive thread names makes debugging
> > > significantly easier. For example, verifying CPU pinning worked as
> > > intended
> > > with ps -T ..., or identifying threads in the Intel VTune profiler.
>
> Why not have the worker threads rename themselves?
That's the way we ended up with, i.e. something like
rte_thread_setname(pthread_self(), "...");
called by each worker.
I guess due to the possibility to set the name by pthread id, and obtain it for
ctrl threads, but not for regular workers, we got the impression of a "missing
piece" in the API. I understand the motivation to keep pthread internals out of
the API, so i guess we can drop the patch.
Thanks
Michael
@@ -114,6 +114,16 @@ rte_lcore_to_socket_id(unsigned int lcore_id)
return lcore_config[lcore_id].socket_id;
}
+int
+rte_lcore_to_thread_id(int lcore_id, pthread_t *thread_id)
+{
+ if (unlikely(lcore_id < 0 || lcore_id >= RTE_MAX_LCORE))
+ return -1;
+
+ *thread_id = lcore_config[lcore_id].thread_id;
+ return 0;
+}
+
static int
socket_id_cmp(const void *a, const void *b)
{
@@ -171,6 +171,20 @@ rte_lcore_to_socket_id(unsigned int lcore_id);
*/
int rte_lcore_to_cpu_id(int lcore_id);
+/**
+ * Get the ID of the thread of the specified lcore
+ *
+ * @param lcore_id
+ * the targeted lcore, which MUST be between 0 and RTE_MAX_LCORE-1.
+ * @param thread_id
+ * the thread id is returned in this argument if lcore_id is in range
+ * @return
+ * 0 on success, or -1 if out of range
+ */
+__rte_experimental
+int
+rte_lcore_to_thread_id(int lcore_id, pthread_t *thread_id);
+
#ifdef RTE_HAS_CPUSET
/**
@@ -440,6 +440,7 @@ EXPERIMENTAL {
rte_thread_detach;
rte_thread_equal;
rte_thread_join;
+ rte_lcore_to_thread_id;
};
INTERNAL {