Re: [PATCH v2] trace-cmd: make it build against musl C library

From: Tzvetomir Stoyanov <tz.stoyanov@gmail.com>
To: Beniamin Sandu <beniaminsandu@gmail.com>
Cc: Linux Trace Devel <linux-trace-devel@vger.kernel.org>
Subject: Re: [PATCH v2] trace-cmd: make it build against musl C library
Date: Mon, 30 Nov 2020 18:24:05 +0200	[thread overview]
Message-ID: <CAPpZLN50X6=8nz3U2M6mk+KqDfxcwdr86d-6bHuV_rkphc1Xhw@mail.gmail.com> (raw)
In-Reply-To: <20201130122755.31000-1-beniaminsandu@gmail.com>

On Mon, Nov 30, 2020 at 2:31 PM Beniamin Sandu <beniaminsandu@gmail.com> 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@gmail.com>

Thanks Beniamin!
There is one minor coding style error, as we are using the kernel's
coding style.
No need to send another version, we will fix it when pulling the patch.
Also, note that we are moving the libtracefs in its own repo:
  https://git.kernel.org/pub/scm/libs/libtrace/libtracefs.git/
I'm going to port the libtracefs part of your patch to that repo too.

Reviewed-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>

> ---
>  .../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                           | 16 +++++++++++-----
>  6 files changed, 27 insertions(+), 5 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..1fce6d0 100644
> --- a/tracecmd/trace-tsync.c
> +++ b/tracecmd/trace-tsync.c
> @@ -104,13 +104,16 @@ 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 (!ret)
> +
> +       if (!ret) {
> +               if (!get_first_cpu(&pin_mask, &mask_size))
> +                       pthread_setaffinity_np(instance->tsync_thread, mask_size, pin_mask);
>                 instance->tsync_thread_running = true;
> +       }
> +
>         if (pin_mask)
>                 CPU_FREE(pin_mask);
>         pthread_attr_destroy(&attrib);
> @@ -243,11 +246,14 @@ 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(!ret) {

There should be a space before the open parenthesis here.

> +               if (!get_first_cpu(&pin_mask, &mask_size))
> +                       pthread_setaffinity_np(*thr_id, mask_size, pin_mask);
> +       }
> +
>         if (pin_mask)
>                 CPU_FREE(pin_mask);
>         pthread_attr_destroy(&attrib);
> --
> 2.25.1
>

-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center