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

[Zhu Lingshan]

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

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>