On Sun, Nov 29, 2020 at 12:04 AM Beniamin Sandu <beniaminsandu@xxxxxxxxx> wrote: > > * add some missing headers and macros > * set pthread affinity using pthread_setaffinity_np after creating the thread > instead of pthread_attr_setaffinity_np (which seems to not be implemented > in musl) > > Tested using https://musl.cc/x86_64-linux-musl-native.tgz > > Signed-off-by: Beniamin Sandu <beniaminsandu@xxxxxxxxx> Hi Beniamin, Thanks for testing trace-cmd with different build environments and sending this patch. I looked at the changes, they look good to me in general. I have two small comments. > --- > lib/trace-cmd/include/private/trace-cmd-private.h | 1 + > lib/trace-cmd/include/trace-cmd-local.h | 1 + > lib/tracefs/include/tracefs-local.h | 12 ++++++++++++ > lib/tracefs/tracefs-events.c | 1 + > tracecmd/include/trace-local.h | 1 + > tracecmd/trace-tsync.c | 11 +++++++---- > 6 files changed, 23 insertions(+), 4 deletions(-) > > diff --git a/lib/trace-cmd/include/private/trace-cmd-private.h b/lib/trace-cmd/include/private/trace-cmd-private.h > index 458760e..a0dac5d 100644 > --- a/lib/trace-cmd/include/private/trace-cmd-private.h > +++ b/lib/trace-cmd/include/private/trace-cmd-private.h > @@ -6,6 +6,7 @@ > #ifndef _TRACE_CMD_PRIVATE_H > #define _TRACE_CMD_PRIVATE_H > > +#include <sys/types.h> > #include "traceevent/event-parse.h" > #include "trace-cmd/trace-cmd.h" > > diff --git a/lib/trace-cmd/include/trace-cmd-local.h b/lib/trace-cmd/include/trace-cmd-local.h > index d0a7365..0cd2744 100644 > --- a/lib/trace-cmd/include/trace-cmd-local.h > +++ b/lib/trace-cmd/include/trace-cmd-local.h > @@ -6,6 +6,7 @@ > #ifndef _TRACE_CMD_LOCAL_H > #define _TRACE_CMD_LOCAL_H > > +#include <byteswap.h> > #include "trace-cmd-private.h" > > /* Can be overridden */ > diff --git a/lib/tracefs/include/tracefs-local.h b/lib/tracefs/include/tracefs-local.h > index 9cc371b..bdbf89e 100644 > --- a/lib/tracefs/include/tracefs-local.h > +++ b/lib/tracefs/include/tracefs-local.h > @@ -13,4 +13,16 @@ void warning(const char *fmt, ...); > int str_read_file(const char *file, char **buffer); > char *trace_append_file(const char *dir, const char *name); > > +#ifndef ACCESSPERMS > +#define ACCESSPERMS (S_IRWXU|S_IRWXG|S_IRWXO) /* 0777 */ > +#endif > + > +#ifndef ALLPERMS > +#define ALLPERMS (S_ISUID|S_ISGID|S_ISVTX|S_IRWXU|S_IRWXG|S_IRWXO) /* 07777 */ > +#endif > + > +#ifndef DEFFILEMODE > +#define DEFFILEMODE (S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP|S_IROTH|S_IWOTH) /* 0666*/ > +#endif > + > #endif /* _TRACE_FS_LOCAL_H */ > diff --git a/lib/tracefs/tracefs-events.c b/lib/tracefs/tracefs-events.c > index 80a25ee..a4e5215 100644 > --- a/lib/tracefs/tracefs-events.c > +++ b/lib/tracefs/tracefs-events.c > @@ -13,6 +13,7 @@ > #include <errno.h> > #include <sys/stat.h> > #include <fcntl.h> > +#include <limits.h> > > #include "kbuffer.h" > #include "tracefs.h" > diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h > index 28d1b4e..85c7e03 100644 > --- a/tracecmd/include/trace-local.h > +++ b/tracecmd/include/trace-local.h > @@ -8,6 +8,7 @@ > > #include <sys/types.h> > #include <dirent.h> /* for DIR */ > +#include <limits.h> > > #include "trace-cmd-private.h" > #include "event-utils.h" > diff --git a/tracecmd/trace-tsync.c b/tracecmd/trace-tsync.c > index e639788..2cbe7f2 100644 > --- a/tracecmd/trace-tsync.c > +++ b/tracecmd/trace-tsync.c > @@ -104,11 +104,13 @@ int tracecmd_host_tsync(struct buffer_instance *instance, > > pthread_attr_init(&attrib); > pthread_attr_setdetachstate(&attrib, PTHREAD_CREATE_JOINABLE); > - if (!get_first_cpu(&pin_mask, &mask_size)) > - pthread_attr_setaffinity_np(&attrib, mask_size, pin_mask); > > ret = pthread_create(&instance->tsync_thread, &attrib, > tsync_host_thread, &instance->tsync); > + > + if (!get_first_cpu(&pin_mask, &mask_size)) > + pthread_setaffinity_np(instance->tsync_thread, mask_size, pin_mask); > + The new affinity should be set only in case pthread_create() does not return any error. This new logic could be moved where the ret is checked for error. > if (!ret) > instance->tsync_thread_running = true; > if (pin_mask) > @@ -243,11 +245,12 @@ unsigned int tracecmd_guest_tsync(char *tsync_protos, > pthread_attr_init(&attrib); > tsync->sync_proto = proto; > pthread_attr_setdetachstate(&attrib, PTHREAD_CREATE_JOINABLE); > - if (!get_first_cpu(&pin_mask, &mask_size)) > - pthread_attr_setaffinity_np(&attrib, mask_size, pin_mask); > > ret = pthread_create(thr_id, &attrib, tsync_agent_thread, tsync); > > + if (!get_first_cpu(&pin_mask, &mask_size)) > + pthread_setaffinity_np(*thr_id, mask_size, pin_mask); > + The same here, the new affinity should not be set in case of pthread_create() failure. > if (pin_mask) > CPU_FREE(pin_mask); > pthread_attr_destroy(&attrib); > -- > 2.25.1 > -- Tzvetomir (Ceco) Stoyanov VMware Open Source Technology Center