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@xxxxxxx> >> --- >> 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>