Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <b57142d5ada4a852836f7c0bcbd9cffc4fb8681e.camel@gmail.com>
Subject: Re: [PATCH 1/3] tools libc_compat: add gettid
From:   Riccardo Mancini <rickyman7@gmail.com>
To:     Arnaldo Carvalho de Melo <acme@kernel.org>
Cc:     Namhyung Kim <namhyung@kernel.org>,
        Ian Rogers <irogers@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>, linux-kernel@vger.kernel.org,
        linux-perf-users@vger.kernel.org
Date:   Mon, 26 Jul 2021 12:43:21 +0200
In-Reply-To: <202107260534.Q3Cd7MYm-lkp@intel.com>
References: <309becd325626e04015ad7b5f6fc701e569f918d.1626966805.git.rickyman7@gmail.com>
         <202107260534.Q3Cd7MYm-lkp@intel.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.40.2 (3.40.2-1.fc34) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Arnaldo,

as reported by the kernel test robot, I forgot to include sys/syscall.h and
unistd.h, which were included by some other headers in perf but is causing
troubles in the bpf selftest.
However, this is not all, since bpf tests are still failing since they do not
use HAVE_* macros, therefore gettid gets defined twice (in libc and
libc_compat.h).
Since, afaict, bpf is no longer using libc_compat.h [1] (perf is be the only
user atm), I would remove its dependency on libc_compat.h.
Furthermore, this made me also think whether the HAVE_* or NEED_* macros are
more suited for this case (reallocarray is using COMPAT_NEED_REALLOCARRAY,
gettid is using HAVE_GETTID). I believe HAVE_* are better since compilation
fails if they are missing or misused (as in this case), while NEED_* failures
may be more subtle (ie. only happening with older versions of libc). Therefore,
I think we should also change COMPAT_NEED_REALLOCARRAY to HAVE_REALLOCARRAY.
What do you think?

[1] https://lore.kernel.org/bpf/20200819013607.3607269-2-andriin@fb.com/

Thanks,
Riccardo

On Mon, 2021-07-26 at 06:04 +0800, kernel test robot wrote:
> Hi Riccardo,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on tip/perf/core]
> [also build test ERROR on linux/master linus/master v5.14-rc2 next-20210723]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:   
> https://github.com/0day-ci/linux/commits/Riccardo-Mancini/tools-add-gettid-to-libc_compat-h/20210722-233601
> base:  
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git c76826a65f50038f0504
> 24365dbf3f97203f8710
> config: x86_64-rhel-8.3-kselftests (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build):
>         #
> https://github.com/0day-ci/linux/commit/42df183984cce4c25932242bbf9133684e9425db
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Riccardo-Mancini/tools-add-gettid-to-
> libc_compat-h/20210722-233601
>         git checkout 42df183984cce4c25932242bbf9133684e9425db
>         # save the attached .config to linux build tree
>         mkdir build_dir
>         make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash -C
> tools/testing/selftests/bpf install
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from main.h:15,
>                     from xlated_dumper.c:14:
>    tools/include/tools/libc_compat.h: In function 'gettid':
>    tools/include/tools/libc_compat.h:24:16: warning: implicit declaration of
> function 'syscall' [-Wimplicit-function-declaration]
>       24 |  return (pid_t)syscall(__NR_gettid);
>          |                ^~~~~~~
> > > tools/include/tools/libc_compat.h:24:24: error: '__NR_gettid' undeclared
> > > (first use in this function)
>       24 |  return (pid_t)syscall(__NR_gettid);
>          |                        ^~~~~~~~~~~
>    tools/include/tools/libc_compat.h:24:24: note: each undeclared identifier is
> reported only once for each function it appears in
> --
>    In file included from main.h:15,
>                     from common.c:27:
> > > tools/include/tools/libc_compat.h:22:21: error: static declaration of
> > > 'gettid' follows non-static declaration
>       22 | static inline pid_t gettid(void)
>          |                     ^~~~~~
>    In file included from /usr/include/unistd.h:1170,
>                     from common.c:15:
>    /usr/include/x86_64-linux-gnu/bits/unistd_ext.h:34:16: note: previous
> declaration of 'gettid' was here
>       34 | extern __pid_t gettid (void) __THROW;
>          |                ^~~~~~
>    In file included from main.h:15,
>                     from common.c:27:
>    tools/include/tools/libc_compat.h: In function 'gettid':
> > > tools/include/tools/libc_compat.h:24:24: error: '__NR_gettid' undeclared
> > > (first use in this function)
>       24 |  return (pid_t)syscall(__NR_gettid);
>          |                        ^~~~~~~~~~~
>    tools/include/tools/libc_compat.h:24:24: note: each undeclared identifier is
> reported only once for each function it appears in
> --
>    In file included from main.h:15,
>                     from btf.c:20:
>    tools/include/tools/libc_compat.h: In function 'gettid':
> > > tools/include/tools/libc_compat.h:24:24: error: '__NR_gettid' undeclared
> > > (first use in this function)
>       24 |  return (pid_t)syscall(__NR_gettid);
>          |                        ^~~~~~~~~~~
>    tools/include/tools/libc_compat.h:24:24: note: each undeclared identifier is
> reported only once for each function it appears in
> 
> 
> vim +/__NR_gettid +24 tools/include/tools/libc_compat.h
> 
>     20  
>     21  #ifndef HAVE_GETTID
>   > 22  static inline pid_t gettid(void)
>     23  {
>   > 24          return (pid_t)syscall(__NR_gettid);
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org