[PATCH] KVM: VMX: don't include '<linux/find.h>' directly

[PATCH] KVM: VMX: don't include '<linux/find.h>' directly

[Wolfram Sang]

The header clearly states that it does not want to be included directly,
only via '<linux/bitmap.h>'. Replace the include accordingly.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/x86/kvm/vmx/posted_intr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/posted_intr.h b/arch/x86/kvm/vmx/posted_intr.h
index 1715d2ab07be..ad9116a99bcc 100644
--- a/arch/x86/kvm/vmx/posted_intr.h
+++ b/arch/x86/kvm/vmx/posted_intr.h
@@ -2,7 +2,7 @@
 #ifndef __KVM_X86_VMX_POSTED_INTR_H
 #define __KVM_X86_VMX_POSTED_INTR_H
 
-#include <linux/find.h>
+#include <linux/bitmap.h>
 #include <asm/posted_intr.h>
 
 void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu);
-- 
2.45.2

Re: [PATCH] KVM: VMX: don't include '<linux/find.h>' directly

[Sean Christopherson]

+Yury and Rasmus

On Tue, Dec 17, 2024, Wolfram Sang wrote:
> The header clearly states that it does not want to be included directly,

I definitely don't object to the KVM change, but the if y'all expect developers
to actually honor the "rule", it needs to have teeth.  As evidenced by a similar
rule in arch/x86/include/asm/bitops.h that also gets ignored, an #error that's
buried under an include guard and triggers on a macro that's never #undef'd is
quite useless.

  $ git grep "include <asm/bitops.h>"
  arch/x86/include/asm/spinlock.h:#include <asm/bitops.h>
  arch/x86/kernel/unwind_frame.c:#include <asm/bitops.h>
  arch/x86/kernel/unwind_guess.c:#include <asm/bitops.h>
  include/linux/bitops.h:#include <asm/bitops.h>
  include/linux/count_zeros.h:#include <asm/bitops.h>
  net/core/net-traces.c:#include <asm/bitops.h>

E.g.

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 262b6596eca5..381ca834985d 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -8,7 +8,11 @@
 #include <linux/bitops.h>
 #include <linux/cleanup.h>
 #include <linux/errno.h>
+
+#define __LINUX_BITMAP_INCLUDE_FIND_H
 #include <linux/find.h>
+#undef  __LINUX_BITMAP_INCLUDE_FIND_H
+
 #include <linux/limits.h>
 #include <linux/string.h>
 #include <linux/types.h>
diff --git a/include/linux/find.h b/include/linux/find.h
index 68685714bc18..d9494ff3126b 100644
--- a/include/linux/find.h
+++ b/include/linux/find.h
@@ -1,11 +1,11 @@
+#ifndef __LINUX_BITMAP_INCLUDE_FIND_H
+#error Please do not include <linux/find.h> directly, include <linux/bitmap.h> instead.
+#endif
+
 /* SPDX-License-Identifier: GPL-2.0 */
 #ifndef __LINUX_FIND_H_
 #define __LINUX_FIND_H_
 
-#ifndef __LINUX_BITMAP_H
-#error only <linux/bitmap.h> can be included directly
-#endif
-
 #include <linux/bitops.h>
 
 unsigned long _find_next_bit(const unsigned long *addr1, unsigned long nbits,

> only via '<linux/bitmap.h>'. Replace the include accordingly.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  arch/x86/kvm/vmx/posted_intr.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx/posted_intr.h b/arch/x86/kvm/vmx/posted_intr.h
> index 1715d2ab07be..ad9116a99bcc 100644
> --- a/arch/x86/kvm/vmx/posted_intr.h
> +++ b/arch/x86/kvm/vmx/posted_intr.h
> @@ -2,7 +2,7 @@
>  #ifndef __KVM_X86_VMX_POSTED_INTR_H
>  #define __KVM_X86_VMX_POSTED_INTR_H
>  
> -#include <linux/find.h>
> +#include <linux/bitmap.h>
>  #include <asm/posted_intr.h>
>  
>  void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu);
> -- 
> 2.45.2
> 

Re: [PATCH] KVM: VMX: don't include '<linux/find.h>' directly

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 633 bytes --]

Hi Sean,

> I definitely don't object to the KVM change, but the if y'all expect developers
> to actually honor the "rule", it needs to have teeth.  As evidenced by a similar
> rule in arch/x86/include/asm/bitops.h that also gets ignored, an #error that's
> buried under an include guard and triggers on a macro that's never #undef'd is
> quite useless.

I went for the minimal change here. But if agreed, we could change
bitmap.h to the pattern spinlock.h is using. Similar to your suggestion,
yet with more readable names IMHO:

#define __LINUX_INSIDE_SPINLOCK_H

...

#undef __LINUX_INSIDE_SPINLOCK_H

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] KVM: VMX: don't include '<linux/find.h>' directly

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

On Tue, Dec 17, 2024 at 08:37:27AM -0800, Sean Christopherson wrote:
> +Yury and Rasmus
> 
> On Tue, Dec 17, 2024, Wolfram Sang wrote:
> > The header clearly states that it does not want to be included directly,
> 
> I definitely don't object to the KVM change, but the if y'all expect developers
> to actually honor the "rule", it needs to have teeth.  As evidenced by a similar
> rule in arch/x86/include/asm/bitops.h that also gets ignored, an #error that's
> buried under an include guard and triggers on a macro that's never #undef'd is
> quite useless.

To follow up, I am willing to do the proposed change. But before sending
this out, it would be nice if the existing broken includes would be
fixed. So, can this patch be applied first?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] KVM: VMX: don't include '<linux/find.h>' directly

[Paolo Bonzini]

Queued, thanks.

Paolo