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

Wolfram Sang posted 1 patch 1 year ago
arch/x86/kvm/vmx/posted_intr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] KVM: VMX: don't include '<linux/find.h>' directly
Posted by Wolfram Sang 1 year ago
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
Posted by Paolo Bonzini 12 months ago
Queued, thanks.

Paolo
Re: [PATCH] KVM: VMX: don't include '<linux/find.h>' directly
Posted by Sean Christopherson 1 year ago
+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
Posted by Wolfram Sang 12 months ago
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?

Re: [PATCH] KVM: VMX: don't include '<linux/find.h>' directly
Posted by Wolfram Sang 1 year ago
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