Re: unnecessary #ifdefs

From: Stefan Hengelein
Date: Wed May 27 2015 - 06:27:55 EST


2015-05-27 2:17 GMT+02:00 Wu, Feng <feng.wu@xxxxxxxxx>:
>> Sent: Wednesday, May 27, 2015 2:19 AM
>> To: Wu, Feng
>> Cc: Valentin Rothberg; Paul Bolle; Andreas Ruprecht; tglx@xxxxxxxxxxxxx;
>> x86@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; rusty@xxxxxxxxxxxxxxx;
>> mingo@xxxxxxxxxx
>> Subject: unnecessary #ifdefs
>>
>> Hi Feng Wu,
>>
>> your commits
>>
>> f6b3c72c23661e55 ("x86/irq: Define a global vector for VT-d
>> Posted-Interrupts")
>> 501b32653ebf49114c ("x86/irq: Show statistics information for
>> posted-interrupts")
>>
>> showed up in linux-next today (i.e. next-20150526).
>>
>> Both commits add "#ifdef CONFIG_HAVE_KVM" blocks to either
>> arch/x86/kernel/irq.c or arch/x86/include/asm/irq.h
>>
>> However, HAVE_KVM is always enabled in x86, since the root option X86
>> always selects HAVE_KVM.
>>
>> How is the policy here, are these blocks inserted in case the "select
>> HAVE_KVM" is removed from X86 someday or did you mean to use
>> CONFIG_KVM?
>>
>
> In fact, some of the "#ifdef CONFIG_HAVE_KVM" have been there for a while,
> it was introduced for a special IPI for posted-interrupts used for KVM. Now we
> need to add other similar IPI for posted-interrupts for KVM, so I just follow the
> existing infrastructure. You can refer to commit " d78f2664832f8d70e36422af9a10e44276dced48 ",
> where "#ifdef CONFIG_HAVE_KVM" was original used.
>
> Thanks,
> Feng

Yes, so i've noticed. However, that doesn't change, that these #ifdefs
and the #ifdefs introduced with d78f2664832f8d70e3642 are unnecessary
since CONFIG_HAVE_KVM is always enabled in X86.
Maybe Yang Zhang can give some insights why they were introduced in
the first place?

Since both files are beneath arch/x86/ they shouldn't be used in
another architecture than X86 and even UML has its own
arch/x86/kernel/irq.c and arch/x86/include/asm/irq.h

Best Regards,
Stefan

>
>>
>> I detected the issue with undertaker-checkpatch [1, 2] running on a
>> bot in Erlangen [3] to make daily checks of commits in linux-next for
>> #ifdef related defects.
>>
>> Best Regards,
>> Stefan
>>
>> [1] https://undertaker.cs.fau.de
>> [2] http://www.linuxplumbersconf.org/2014/ocw/proposals/1863
>> [3] https://cados.cs.fau.de
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/