Re: [PATCH] tools: Fixed MIPS builds due to struct flock re-definition

Florian Fainelli
2022-07-19T19:05:30Z

On 7/19/2022 12:42 AM, Arnd Bergmann wrote:
> On Fri, Jul 15, 2022 at 8:55 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> Building perf for MIPS failed after 9f79b8b72339 ("uapi: simplify
>> __ARCH_FLOCK{,64}_PAD a little") with the following error:
>>
>> CC
>> /home/fainelli/work/buildroot/output/bmips/build/linux-custom/tools/perf/trace/beauty/fcntl.o
>> In file included from
>> ../../../../host/mipsel-buildroot-linux-gnu/sysroot/usr/include/asm/fcntl.h:77,
>> from ../include/uapi/linux/fcntl.h:5,
>> from trace/beauty/fcntl.c:10:
>> ../include/uapi/asm-generic/fcntl.h:188:8: error: redefinition of
>> 'struct flock'
>> struct flock {
>> ^~~~~
>> In file included from ../include/uapi/linux/fcntl.h:5,
>> from trace/beauty/fcntl.c:10:
>> ../../../../host/mipsel-buildroot-linux-gnu/sysroot/usr/include/asm/fcntl.h:63:8:
>> note: originally defined here
>> struct flock {
>> ^~~~~
>>
>> This is due to the local copy under
>> tools/include/uapi/asm-generic/fcntl.h including the toolchain's kernel
>> headers which already define 'struct flock' and define
>> HAVE_ARCH_STRUCT_FLOCK to future inclusions make a decision as to
>> whether re-defining 'struct flock' is appropriate or not.
>>
>> Make sure what do not re-define 'struct flock'
>> when HAVE_ARCH_STRUCT_FLOCK is already defined.
>>
>> Fixes: 9f79b8b72339 ("uapi: simplify __ARCH_FLOCK{,64}_PAD a little")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>> tools/include/uapi/asm-generic/fcntl.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
>> index 0197042b7dfb..312881aa272b 100644
>> --- a/tools/include/uapi/asm-generic/fcntl.h
>> +++ b/tools/include/uapi/asm-generic/fcntl.h
>> @@ -185,6 +185,7 @@ struct f_owner_ex {
>>
>> #define F_LINUX_SPECIFIC_BASE 1024
>>
>> +#ifndef HAVE_ARCH_STRUCT_FLOCK
>> struct flock {
>> short l_type;
>> short l_whence;
>> @@ -209,5 +210,6 @@ struct flock64 {
>> __ARCH_FLOCK64_PAD
>> #endif
>> };
>> +#endif /* HAVE_ARCH_STRUCT_FLOCK */
>>
>
> I applied this to the asm-generic tree, but now I'm having second thoughts, as
> this only changes the tools/include/ version but not the version we ship to user
> space. Normally these are meant to be kept in sync.

Thanks! Just to be clear, applying just your patch is not enough as the
original build issue is still present, so we would need my change plus
yours, I think that is what you intended but just wanted to double
confirm. On a side note your tree at:

https://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git/refs/heads

does not appear to have it included/pushed out yet, should I be looking
at another git tree?

>
> It appears that commit 306f7cc1e906 ("uapi: always define
> F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h") already caused
> them to diverge, presumably the uapi version here is correct and we
> forgot to adapt the tools version at some point. There are also some
> non-functional differences from older patches.
>
> I think the correct fix to address the problem in both versions and
> get them back into sync would be something like the patch below.
> I have done zero testing on it though.
>
> Christoph and Florian, any other suggestions?

This works for me with my patch plus your patch in the following
configurations:

- MIPS toolchain with kernel-headers 4.1.x
- MIPS toolchain with kernel headers using my patch plus your patch

Tested-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian