[RESEND] amdkfd: always include uapi header in priv.h

[Felix Kuehling]

On 2024-10-31 22:15, Zhu Lingshan wrote:
> On 10/31/2024 11:27 PM, Felix Kuehling wrote:
>> On 2024-10-31 6:47, Zhu Lingshan wrote:
>>> The header usr/linux/kfd_ioctl.h is a duplicate of uapi/linux/kfd_ioctl.h.
>> I don't see usr/linux/kfd_ioctl.h. Which branch are you looking at?
> The mainline master branch:
> https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/amd/amdkfd/kfd_priv.h#L35
> #include <linux/kfd_ioctl.h>
>>
>>> And it is actually not a file in the source code tree.
>> If it's a file that only there on your local working tree, maybe just clean up your working tree.
> Yes, this is a file on the local working tree, it is generated when build, it is  not in source code tree.
> The is even no folder usr/include/linux before building.
> 
> For a quick verify:
> 1) it is not a source code file from mainline:
> $git blame ./usr/include/linux/kfd_ioctl.h
> fatal: no such path 'usr/include/linux/kfd_ioctl.h' in HEAD
> 
> 2) it is generated when build:
> remove usr/include/linux/kfd_ioctl.h, then build.
> If we remove a required header, kfd would not
> build and there will be error messages.
> 
> but here we will see these lines:
> HDRINST usr/include/linux/kfd_ioctl.h
> HDRTEST usr/include/linux/kfd_ioctl.h,
> no build errors, and usr/include/linux/kfd_ioctl.h is
> generated by duplicating the uapi one.
> 
> 
> 2) linux/kfd_ioctl.h is usr/include/linux/kfd_ioctl.h
> add random characters in usr/include/linux/kfd_ioctl.h, then build
> we will see errors like this:
> 
> In file included from <command-line>:
> ./usr/include/linux/kfd_ioctl.h:65:9: error: expected ‘;’ before ‘struct’
>    65 | abcd1234
>       |         ^
>       |         ;
>    66 |
>    67 | struct kfd_ioctl_create_queue_args {
>       | ~~~~~~
>>
>>> Ideally, the usr version should be updated whenever the source code is recompiled.
>> If the usr version is not in the git repository, it doesn't need to be updated. I don't know where this is coming from on your local tree.
> the usr one would be installed to the system when run make install or install_headers,
> it is for user space, we should include the uapi one instead of usr one in our source code files

I did not see the folder in my tree because I build with O=... so usr/include/linux/kfd_ioctl.h shows up in my build output tree.

The extra copy of the user mode headers seems to be an artifact of the Linux kernel build system. The fact that the build picks up user mode headers from <OUT>/usr/include/... seems intentional. If your tree has an outdated version of kfd_ioctl.h there, it's probably something broken with your build tree. Maybe broken file timestamps. It should be easy to fix with a "make mrproper" to force it to update all the build artifacts.

I still don't think we need to change the code to fix a problem specific to your build system.

Regards,
  Felix

> 
> Thanks
> Lingshan
>>
>> Regards,
>>   Felix
>>
>>> However, I have noticed a discrepancy between the two headers
>>> even after rebuilding the kernel.
>>>
>>> This commit modifies kfd_priv.h to always include the header from uapi to ensure
>>> the latest changes are reflected. We should always include the source
>>> code header other than the duplication.
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu at amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> index 26e48fdc8728..78de7ac09e8a 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> @@ -32,7 +32,7 @@
>>>  #include <linux/atomic.h>
>>>  #include <linux/workqueue.h>
>>>  #include <linux/spinlock.h>
>>> -#include <linux/kfd_ioctl.h>
>>> +#include <uapi/linux/kfd_ioctl.h>
>>>  #include <linux/idr.h>
>>>  #include <linux/kfifo.h>
>>>  #include <linux/seq_file.h>
>