* [PATCH v1 0/2] x86: simplify UP APIC conditions @ 2015-03-11 23:10 Luis R. Rodriguez 2015-03-11 23:10 ` [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC Luis R. Rodriguez 2015-03-11 23:10 ` [PATCH v1 2/2] x86: kconfig: remove X86_UP_APIC Luis R. Rodriguez 0 siblings, 2 replies; 9+ messages in thread From: Luis R. Rodriguez @ 2015-03-11 23:10 UTC (permalink / raw) To: rientjes, bhelgaas, mingo, tglx, hpa, pure.logic, jgross, luto, andy.shevchenko, thomas.petazzoni, JBeulich, bp Cc: linux-kernel, linux-pci, x86, Luis R. Rodriguez From: "Luis R. Rodriguez" <mcgrof@suse.com> There is no need for UP APIC Kconfig entries given x86 is not in a state of flux for 32-bit and the options really don't have major impact on kernel size or performance. Things can also be expressed very clearly in a more simplified way, doing that also removes all the UP APIC kconfig clutter. Luis R. Rodriguez (2): x86: kconfig: remove X86_UP_IOAPIC x86: kconfig: remove X86_UP_APIC arch/x86/Kconfig | 30 ++---------------------------- drivers/pci/Kconfig | 2 ++ 2 files changed, 4 insertions(+), 28 deletions(-) -- 2.3.2.209.gd67f9d5.dirty ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC 2015-03-11 23:10 [PATCH v1 0/2] x86: simplify UP APIC conditions Luis R. Rodriguez @ 2015-03-11 23:10 ` Luis R. Rodriguez 2015-03-12 1:19 ` Bryan O'Donoghue 2015-03-12 5:36 ` David Rientjes 2015-03-11 23:10 ` [PATCH v1 2/2] x86: kconfig: remove X86_UP_APIC Luis R. Rodriguez 1 sibling, 2 replies; 9+ messages in thread From: Luis R. Rodriguez @ 2015-03-11 23:10 UTC (permalink / raw) To: rientjes, bhelgaas, mingo, tglx, hpa, pure.logic, jgross, luto, andy.shevchenko, thomas.petazzoni, JBeulich, bp Cc: linux-kernel, linux-pci, x86, Luis R. Rodriguez, Ingo Molnar From: "Luis R. Rodriguez" <mcgrof@suse.com> X86_UP_IOAPIC is a way so that 32-bit UP systems can enable X86_IOAPIC. X86_UP_IOAPIC is only as a visible user option if you are on a 32-bit system but have X86_UP_APIC enabled. X86_UP_APIC will be enabled by force if you have PCI_MSI on 32-bit systems now, X86_UP_APIC will now only be user selectable if you didn't have PCI_MSI enabled and are also not on a X86_32_NON_STANDARD system. Bryan's original patch (refactored commit log in commit 38a1dfda) [0] describes that Intel CE, Intel MID and Intel Quark are all 32-bit uniprocessor systems with IO-APICs, the code change however only *re-enabled* UP_IOAPIC as an *option* when PCI_MSI was enabled, but given that: 1) enabling X86_IOAPIC is the real end goal here 2) enabling X86_IOAPIC only increases the kernel only by 12064 bytes (~12 KiB) 3) enabling X86_IOAPIC will in no way slow down your kernel Let's make a compromise for 32-bit systems and always enable X86_IOAPIC when X86_UP_IOAPIC is enabled as 32-bit systems are not in a state of flux and the price for the size is small with no performance impact. Using: export ARCH=i386 make allnoconfig --> Enabling PCI_MSI make localyesconfig With X86_IO_APIC: mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage 734608 arch/x86/boot/bzImage Without X86_IO_APIC: mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage 722544 arch/x86/boot/bzImage [0] https://lkml.org/lkml/2015/1/22/718 Cc: David Rientjes <rientjes@google.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Bryan O'Donoghue <pure.logic@nexus-software.ie> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Ingo Molnar <mingo@kernel.org> Cc: Borislav Petkov <bp@suse.de> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Jan Beulich <JBeulich@suse.com> Cc: Juergen Gross <jgross@suse.com> Cc: linux-pci@vger.kernel.org Cc: x86@kernel.org Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com> --- arch/x86/Kconfig | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 110f6ae..b17a8ea 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -899,6 +899,7 @@ config X86_UP_APIC bool "Local APIC support on uniprocessors" if !PCI_MSI default PCI_MSI depends on X86_32 && !SMP && !X86_32_NON_STANDARD + select X86_IO_APIC ---help--- A local APIC (Advanced Programmable Interrupt Controller) is an integrated interrupt controller in the CPU. If you have a single-CPU @@ -909,18 +910,6 @@ config X86_UP_APIC performance counters), and the NMI watchdog which detects hard lockups. -config X86_UP_IOAPIC - bool "IO-APIC support on uniprocessors" - depends on X86_UP_APIC - ---help--- - An IO-APIC (I/O Advanced Programmable Interrupt Controller) is an - SMP-capable replacement for PC-style interrupt controllers. Most - SMP systems and many recent uniprocessor systems have one. - - If you have a single-CPU system with an IO-APIC, you can say Y here - to use it. If you say Y here even though your machine doesn't have - an IO-APIC, then the kernel will still run with no slowdown at all. - config X86_LOCAL_APIC def_bool y depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI @@ -928,7 +917,7 @@ config X86_LOCAL_APIC config X86_IO_APIC def_bool y - depends on X86_LOCAL_APIC || X86_UP_IOAPIC + depends on X86_LOCAL_APIC select IRQ_DOMAIN config X86_REROUTE_FOR_BROKEN_BOOT_IRQS -- 2.3.2.209.gd67f9d5.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC 2015-03-11 23:10 ` [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC Luis R. Rodriguez @ 2015-03-12 1:19 ` Bryan O'Donoghue 2015-03-12 15:35 ` Luis R. Rodriguez 2015-03-12 5:36 ` David Rientjes 1 sibling, 1 reply; 9+ messages in thread From: Bryan O'Donoghue @ 2015-03-12 1:19 UTC (permalink / raw) To: Luis R. Rodriguez, rientjes, bhelgaas, mingo, tglx, hpa, jgross, luto, andy.shevchenko, thomas.petazzoni, JBeulich, bp Cc: linux-kernel, linux-pci, x86, Luis R. Rodriguez, Ingo Molnar On 11/03/15 23:10, Luis R. Rodriguez wrote: ACK the concept - the logic to compile up APIC support is circuitous to say the least. Personally think we should just always compile up the APIC code if the arch declares support and let the bootstrap code interrogate CPUID. Who in 2015 is really running a system without an APIC/IO-APIC and tip-of-tree Linux and does that one user care about adding 12k to her kernel ? I suspect not and in any case can force the APIC off with a command line argument > @@ -899,6 +899,7 @@ config X86_UP_APIC > bool "Local APIC support on uniprocessors" if !PCI_MSI Tried to apply this to torvalds-master to test :( Should it ? Which branch are you on here ? Applying: x86: kconfig: remove X86_UP_IOAPIC error: patch failed: arch/x86/Kconfig:899 error: arch/x86/Kconfig: patch does not apply Patch failed at 0001 x86: kconfig: remove X86_UP_IOAPIC ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC 2015-03-12 1:19 ` Bryan O'Donoghue @ 2015-03-12 15:35 ` Luis R. Rodriguez 0 siblings, 0 replies; 9+ messages in thread From: Luis R. Rodriguez @ 2015-03-12 15:35 UTC (permalink / raw) To: Bryan O'Donoghue Cc: Luis R. Rodriguez, rientjes, bhelgaas, mingo, tglx, hpa, jgross, luto, andy.shevchenko, thomas.petazzoni, JBeulich, bp, linux-kernel, linux-pci, x86, Ingo Molnar On Thu, Mar 12, 2015 at 01:19:14AM +0000, Bryan O'Donoghue wrote: > On 11/03/15 23:10, Luis R. Rodriguez wrote: > > ACK the concept - the logic to compile up APIC support is circuitous > to say the least. It took me a while to grok this and indeed the goal was to make it much simpler to read, but at the same time to see if we can reach a compromise to simplify it for 32-bit. > Personally think we should just always compile up the APIC code if > the arch declares support and let the bootstrap code interrogate > CPUID. This would be the *next* level of compromise to make, I felt comfortable in raising the size compromise question for 32-bit but its not clear to me if this is a general question which we can address for all x86. There is indeed no performance pentalty for both so the question comes down to tex size increase, and its why I provided the numbers. My preference was to leave the optimization question for all x86 as a rather secondary question *iff* we can agree on something for 32-bit. > Who in 2015 is really running a system without an > APIC/IO-APIC and tip-of-tree Linux and does that one user care about > adding 12k to her kernel ? I suspect not and in any case can force > the APIC off with a command line argument I also figured this was the case, but figured it was safer to pose the question for 32-bit. If indeed folks who produce the hardware can conclude the size increase is reasonable for all platforms given no performance penalty then we can surely keep this even simpler -- I think its safer to ask this question for 32-bit and leave only the larger picture questoin as an evolutionairy question. > >@@ -899,6 +899,7 @@ config X86_UP_APIC > > bool "Local APIC support on uniprocessors" if !PCI_MSI > > Tried to apply this to torvalds-master to test :( Should it ? Which > branch are you on here ? > > Applying: x86: kconfig: remove X86_UP_IOAPIC > error: patch failed: arch/x86/Kconfig:899 > error: arch/x86/Kconfig: patch does not apply > Patch failed at 0001 x86: kconfig: remove X86_UP_IOAPIC linux-next tag next-20150311 Luis ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC 2015-03-11 23:10 ` [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC Luis R. Rodriguez 2015-03-12 1:19 ` Bryan O'Donoghue @ 2015-03-12 5:36 ` David Rientjes 2015-03-12 15:26 ` Luis R. Rodriguez 1 sibling, 1 reply; 9+ messages in thread From: David Rientjes @ 2015-03-12 5:36 UTC (permalink / raw) To: Luis R. Rodriguez Cc: bhelgaas, mingo, tglx, hpa, pure.logic, jgross, luto, andy.shevchenko, thomas.petazzoni, JBeulich, bp, linux-kernel, linux-pci, x86, Luis R. Rodriguez, Ingo Molnar On Wed, 11 Mar 2015, Luis R. Rodriguez wrote: > From: "Luis R. Rodriguez" <mcgrof@suse.com> > > X86_UP_IOAPIC is a way so that 32-bit UP systems can enable > X86_IOAPIC. X86_UP_IOAPIC is only as a visible user option if > you are on a 32-bit system but have X86_UP_APIC enabled. X86_UP_APIC > will be enabled by force if you have PCI_MSI on 32-bit systems > now, X86_UP_APIC will now only be user selectable if you didn't > have PCI_MSI enabled and are also not on a X86_32_NON_STANDARD > system. Bryan's original patch (refactored commit log in commit > 38a1dfda) [0] describes that Intel CE, Intel MID and Intel Quark > are all 32-bit uniprocessor systems with IO-APICs, the code change > however only *re-enabled* UP_IOAPIC as an *option* when PCI_MSI > was enabled, but given that: > > 1) enabling X86_IOAPIC is the real end goal here > 2) enabling X86_IOAPIC only increases the kernel only by 12064 bytes (~12 KiB) > 3) enabling X86_IOAPIC will in no way slow down your kernel > > Let's make a compromise for 32-bit systems and always enable X86_IOAPIC > when X86_UP_IOAPIC is enabled as 32-bit systems are not in a state > of flux and the price for the size is small with no performance impact. > > Using: > > export ARCH=i386 > make allnoconfig > --> Enabling PCI_MSI > make localyesconfig > > With X86_IO_APIC: > mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage > 734608 arch/x86/boot/bzImage > > Without X86_IO_APIC: > mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage > 722544 arch/x86/boot/bzImage > 1.6% increase. > [0] https://lkml.org/lkml/2015/1/22/718 > > Cc: David Rientjes <rientjes@google.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Bryan O'Donoghue <pure.logic@nexus-software.ie> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Borislav Petkov <bp@suse.de> > Cc: H. Peter Anvin <hpa@zytor.com> > Cc: Jan Beulich <JBeulich@suse.com> > Cc: Juergen Gross <jgross@suse.com> > Cc: linux-pci@vger.kernel.org > Cc: x86@kernel.org > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com> > --- > arch/x86/Kconfig | 15 ++------------- > 1 file changed, 2 insertions(+), 13 deletions(-) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 110f6ae..b17a8ea 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -899,6 +899,7 @@ config X86_UP_APIC > bool "Local APIC support on uniprocessors" if !PCI_MSI > default PCI_MSI > depends on X86_32 && !SMP && !X86_32_NON_STANDARD > + select X86_IO_APIC > ---help--- > A local APIC (Advanced Programmable Interrupt Controller) is an > integrated interrupt controller in the CPU. If you have a single-CPU > @@ -909,18 +910,6 @@ config X86_UP_APIC > performance counters), and the NMI watchdog which detects hard > lockups. > > -config X86_UP_IOAPIC > - bool "IO-APIC support on uniprocessors" > - depends on X86_UP_APIC > - ---help--- > - An IO-APIC (I/O Advanced Programmable Interrupt Controller) is an > - SMP-capable replacement for PC-style interrupt controllers. Most > - SMP systems and many recent uniprocessor systems have one. > - > - If you have a single-CPU system with an IO-APIC, you can say Y here > - to use it. If you say Y here even though your machine doesn't have > - an IO-APIC, then the kernel will still run with no slowdown at all. > - > config X86_LOCAL_APIC > def_bool y > depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI > @@ -928,7 +917,7 @@ config X86_LOCAL_APIC > > config X86_IO_APIC > def_bool y > - depends on X86_LOCAL_APIC || X86_UP_IOAPIC > + depends on X86_LOCAL_APIC > select IRQ_DOMAIN > > config X86_REROUTE_FOR_BROKEN_BOOT_IRQS I think it would be best to remove the "select" so the "depends" for both config options won't diverge in the future. This should be equivalent, right? diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -909,18 +909,6 @@ config X86_UP_APIC performance counters), and the NMI watchdog which detects hard lockups. -config X86_UP_IOAPIC - bool "IO-APIC support on uniprocessors" - depends on X86_UP_APIC - ---help--- - An IO-APIC (I/O Advanced Programmable Interrupt Controller) is an - SMP-capable replacement for PC-style interrupt controllers. Most - SMP systems and many recent uniprocessor systems have one. - - If you have a single-CPU system with an IO-APIC, you can say Y here - to use it. If you say Y here even though your machine doesn't have - an IO-APIC, then the kernel will still run with no slowdown at all. - config X86_LOCAL_APIC def_bool y depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI @@ -928,7 +916,7 @@ config X86_LOCAL_APIC config X86_IO_APIC def_bool y - depends on X86_LOCAL_APIC || X86_UP_IOAPIC + depends on X86_LOCAL_APIC || X86_UP_APIC select IRQ_DOMAIN config X86_REROUTE_FOR_BROKEN_BOOT_IRQS And then the second patch adds a 3.8% increase on top of this (and the two "select" statements in that patch shouldn't be necessary, both X86_LOCAL_APIC and X86_IO_APIC are def_bool y for PCI_MSI configs). If these are just cleanup patches, I'm not sure I understand why a considerable kernel text size increase is worth it. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC 2015-03-12 5:36 ` David Rientjes @ 2015-03-12 15:26 ` Luis R. Rodriguez 0 siblings, 0 replies; 9+ messages in thread From: Luis R. Rodriguez @ 2015-03-12 15:26 UTC (permalink / raw) To: David Rientjes Cc: Luis R. Rodriguez, bhelgaas, mingo, tglx, hpa, pure.logic, jgross, luto, andy.shevchenko, thomas.petazzoni, JBeulich, bp, linux-kernel, linux-pci, x86, Ingo Molnar On Wed, Mar 11, 2015 at 10:36:41PM -0700, David Rientjes wrote: > On Wed, 11 Mar 2015, Luis R. Rodriguez wrote: > > > From: "Luis R. Rodriguez" <mcgrof@suse.com> > > > > X86_UP_IOAPIC is a way so that 32-bit UP systems can enable > > X86_IOAPIC. X86_UP_IOAPIC is only as a visible user option if > > you are on a 32-bit system but have X86_UP_APIC enabled. X86_UP_APIC > > will be enabled by force if you have PCI_MSI on 32-bit systems > > now, X86_UP_APIC will now only be user selectable if you didn't > > have PCI_MSI enabled and are also not on a X86_32_NON_STANDARD > > system. Bryan's original patch (refactored commit log in commit > > 38a1dfda) [0] describes that Intel CE, Intel MID and Intel Quark > > are all 32-bit uniprocessor systems with IO-APICs, the code change > > however only *re-enabled* UP_IOAPIC as an *option* when PCI_MSI > > was enabled, but given that: > > > > 1) enabling X86_IOAPIC is the real end goal here > > 2) enabling X86_IOAPIC only increases the kernel only by 12064 bytes (~12 KiB) > > 3) enabling X86_IOAPIC will in no way slow down your kernel > > > > Let's make a compromise for 32-bit systems and always enable X86_IOAPIC > > when X86_UP_IOAPIC is enabled as 32-bit systems are not in a state > > of flux and the price for the size is small with no performance impact. > > > > Using: > > > > export ARCH=i386 > > make allnoconfig > > --> Enabling PCI_MSI > > make localyesconfig > > > > With X86_IO_APIC: > > mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage > > 734608 arch/x86/boot/bzImage > > > > Without X86_IO_APIC: > > mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage > > 722544 arch/x86/boot/bzImage > > > > 1.6% increase. Indeed, but we are talking about x86-32 bit and the question these two patches bring up is -- is the gains of the cleanup from the removal of these two worth the compromise? I think it is, but if there is active interest in keeping 32-bit x86 builds tiny I understand if this is not desirable. > > [0] https://lkml.org/lkml/2015/1/22/718 > > > > Cc: David Rientjes <rientjes@google.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Bryan O'Donoghue <pure.logic@nexus-software.ie> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Andy Shevchenko <andy.shevchenko@gmail.com> > > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > > Cc: Ingo Molnar <mingo@kernel.org> > > Cc: Borislav Petkov <bp@suse.de> > > Cc: H. Peter Anvin <hpa@zytor.com> > > Cc: Jan Beulich <JBeulich@suse.com> > > Cc: Juergen Gross <jgross@suse.com> > > Cc: linux-pci@vger.kernel.org > > Cc: x86@kernel.org > > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com> > > --- > > arch/x86/Kconfig | 15 ++------------- > > 1 file changed, 2 insertions(+), 13 deletions(-) > > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > > index 110f6ae..b17a8ea 100644 > > --- a/arch/x86/Kconfig > > +++ b/arch/x86/Kconfig > > @@ -899,6 +899,7 @@ config X86_UP_APIC > > bool "Local APIC support on uniprocessors" if !PCI_MSI > > default PCI_MSI > > depends on X86_32 && !SMP && !X86_32_NON_STANDARD > > + select X86_IO_APIC > > ---help--- > > A local APIC (Advanced Programmable Interrupt Controller) is an > > integrated interrupt controller in the CPU. If you have a single-CPU > > @@ -909,18 +910,6 @@ config X86_UP_APIC > > performance counters), and the NMI watchdog which detects hard > > lockups. > > > > -config X86_UP_IOAPIC > > - bool "IO-APIC support on uniprocessors" > > - depends on X86_UP_APIC > > - ---help--- > > - An IO-APIC (I/O Advanced Programmable Interrupt Controller) is an > > - SMP-capable replacement for PC-style interrupt controllers. Most > > - SMP systems and many recent uniprocessor systems have one. > > - > > - If you have a single-CPU system with an IO-APIC, you can say Y here > > - to use it. If you say Y here even though your machine doesn't have > > - an IO-APIC, then the kernel will still run with no slowdown at all. > > - > > config X86_LOCAL_APIC > > def_bool y > > depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI > > @@ -928,7 +917,7 @@ config X86_LOCAL_APIC > > > > config X86_IO_APIC > > def_bool y > > - depends on X86_LOCAL_APIC || X86_UP_IOAPIC > > + depends on X86_LOCAL_APIC > > select IRQ_DOMAIN > > > > config X86_REROUTE_FOR_BROKEN_BOOT_IRQS > > I think it would be best to remove the "select" so the "depends" for both > config options won't diverge in the future. This should be equivalent, > right? Do you mean remove the "select IRQ_DOMAIN" ? If so the "select" does not imply "depends" though, it simply will peg it on if its own dependencies are met. > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -909,18 +909,6 @@ config X86_UP_APIC > performance counters), and the NMI watchdog which detects hard > lockups. > > -config X86_UP_IOAPIC > - bool "IO-APIC support on uniprocessors" > - depends on X86_UP_APIC > - ---help--- > - An IO-APIC (I/O Advanced Programmable Interrupt Controller) is an > - SMP-capable replacement for PC-style interrupt controllers. Most > - SMP systems and many recent uniprocessor systems have one. > - > - If you have a single-CPU system with an IO-APIC, you can say Y here > - to use it. If you say Y here even though your machine doesn't have > - an IO-APIC, then the kernel will still run with no slowdown at all. > - > config X86_LOCAL_APIC > def_bool y > depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI > @@ -928,7 +916,7 @@ config X86_LOCAL_APIC > > config X86_IO_APIC > def_bool y > - depends on X86_LOCAL_APIC || X86_UP_IOAPIC > + depends on X86_LOCAL_APIC || X86_UP_APIC > select IRQ_DOMAIN > > config X86_REROUTE_FOR_BROKEN_BOOT_IRQS > > And then the second patch adds a 3.8% increase on top of this (and the two > "select" statements in that patch shouldn't be necessary, both > X86_LOCAL_APIC and X86_IO_APIC are def_bool y for PCI_MSI configs). This is one way to express that and the trick is the depends are carried over this way. > If these are just cleanup patches, I'm not sure I understand why a > considerable kernel text size increase is worth it. The patches pose the question if the cleanup of the removal of both is worth the compromise to always enable thes two for PCI_MSI and remove the option to enable/disable it on platform where the depends match out. The later would also imply that platforms that need it will need to ensure its components select them, but historically it seems we can only deduce these both are required for PCI_MSI 32-bit platforms. Luis ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 2/2] x86: kconfig: remove X86_UP_APIC 2015-03-11 23:10 [PATCH v1 0/2] x86: simplify UP APIC conditions Luis R. Rodriguez 2015-03-11 23:10 ` [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC Luis R. Rodriguez @ 2015-03-11 23:10 ` Luis R. Rodriguez 2015-03-12 8:42 ` Jan Beulich [not found] ` <55015F690200007800068D86@suse.com> 1 sibling, 2 replies; 9+ messages in thread From: Luis R. Rodriguez @ 2015-03-11 23:10 UTC (permalink / raw) To: rientjes, bhelgaas, mingo, tglx, hpa, pure.logic, jgross, luto, andy.shevchenko, thomas.petazzoni, JBeulich, bp Cc: linux-kernel, linux-pci, x86, Luis R. Rodriguez, Ingo Molnar From: "Luis R. Rodriguez" <mcgrof@suse.com> X86_UP_APIC is used for two reasons on 32-bit systems: 1) set a series of dependencies under which we would like to express we want X86_LOCAL_APIC enabled 2) under the above conditions if PCI_MSI is enabled always force X86_LOCAL_APIC to be enabled 3) Let users opt in or out of X86_LOCAL_APIC if PCI_MSI is not enabled We don't really need a Kconfig entry to address 1) for 2) and 3) as X86_LOCAL_APIC already has the dependencies which we wish to match. Instead since 2) is desirable we can just select X86_LOCAL_APIC on PCI_MSI and it will be enabled when the dependencies are met. The only thing we loose with this then is letting users elect to enable or not X86_LOCAL_APIC on UP 32-bit systems but since: a) enabling X86_LOCAL_APIC will in no way slow down your kernel b) enabling X86_LOCAL_APIC only increases your kernel by 19264 bytes (19 KiB) c) x86 is not in a state of flux We can compromise here and just always enable X86_LOCAL_APIC when PCI_MSI is enabled. Using: (a hacked up patch to enable X86_LOCAL_APIC any time) export ARCH=i386 make allnoconfig --> Enabling or disabling X86_LOCAL_APIC make localyesconfig With X86_LOCAL_APIC: mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage 508496 arch/x86/boot/bzImage With X86_LOCAL_APIC: mcgrof@ergon ~/linux-next (git::master)$ du -b arch/x86/boot/bzImage 489232 arch/x86/boot/bzImage Cc: David Rientjes <rientjes@google.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Bryan O'Donoghue <pure.logic@nexus-software.ie> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Cc: Ingo Molnar <mingo@kernel.org> Cc: Borislav Petkov <bp@suse.de> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Jan Beulich <JBeulich@suse.com> Cc: Juergen Gross <jgross@suse.com> Cc: linux-pci@vger.kernel.org Cc: x86@kernel.org Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com> --- arch/x86/Kconfig | 17 +---------------- drivers/pci/Kconfig | 2 ++ 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index b17a8ea..37d3e6e 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -895,24 +895,9 @@ config UP_LATE_INIT def_bool y depends on !SMP && X86_LOCAL_APIC -config X86_UP_APIC - bool "Local APIC support on uniprocessors" if !PCI_MSI - default PCI_MSI - depends on X86_32 && !SMP && !X86_32_NON_STANDARD - select X86_IO_APIC - ---help--- - A local APIC (Advanced Programmable Interrupt Controller) is an - integrated interrupt controller in the CPU. If you have a single-CPU - system which has a processor with a local APIC, you can say Y here to - enable and use it. If you say Y here even though your machine doesn't - have a local APIC, then the kernel will still run with no slowdown at - all. The local APIC supports CPU-generated self-interrupts (timer, - performance counters), and the NMI watchdog which detects hard - lockups. - config X86_LOCAL_APIC def_bool y - depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI + depends on X86_64 || SMP || X86_32_NON_STANDARD || PCI_MSI select GENERIC_IRQ_LEGACY_ALLOC_HWIRQ config X86_IO_APIC diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 7a8f1c5..fa9739e 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -5,6 +5,8 @@ config PCI_MSI bool "Message Signaled Interrupts (MSI and MSI-X)" depends on PCI select GENERIC_MSI_IRQ + select X86_LOCAL_APIC + select X86_IO_APIC help This allows device drivers to enable MSI (Message Signaled Interrupts). Message Signaled Interrupts enable a device to -- 2.3.2.209.gd67f9d5.dirty ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 2/2] x86: kconfig: remove X86_UP_APIC 2015-03-11 23:10 ` [PATCH v1 2/2] x86: kconfig: remove X86_UP_APIC Luis R. Rodriguez @ 2015-03-12 8:42 ` Jan Beulich [not found] ` <55015F690200007800068D86@suse.com> 1 sibling, 0 replies; 9+ messages in thread From: Jan Beulich @ 2015-03-12 8:42 UTC (permalink / raw) To: Luis R. Rodriguez Cc: luto, thomas.petazzoni, andy.shevchenko, bhelgaas, rientjes, Ingo Molnar, x86, tglx, pure.logic, mingo, Juergen Gross, Luis Rodriguez, bp, linux-kernel, linux-pci, hpa >>> On 12.03.15 at 00:10, <mcgrof@do-not-panic.com> wrote: > config X86_LOCAL_APIC > def_bool y > - depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI > + depends on X86_64 || SMP || X86_32_NON_STANDARD || PCI_MSI I.e. building a 32-bit kernel with APIC support but with !SMP, !PCI_MSI, and !X86_32_NON_STANDARD will now be impossible. Surely not what the patch description says. > --- a/drivers/pci/Kconfig > +++ b/drivers/pci/Kconfig > @@ -5,6 +5,8 @@ config PCI_MSI > bool "Message Signaled Interrupts (MSI and MSI-X)" > depends on PCI > select GENERIC_MSI_IRQ > + select X86_LOCAL_APIC > + select X86_IO_APIC I don't see the need for the latter - MSI specifically works without any IO-APIC interaction. And for the former you should decide which way you want it - PCI_MSI select X86_LOCAL_APIC (probably the right thing in x86, but surely wrong everwhere else, i.e. this at least needs a condition tagged onto it) or X86_LOCAL_APIC depend on PCI_MSI; in no case should this be a forward _and_ reverse dependency. Jan ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <55015F690200007800068D86@suse.com>]
* Re: [PATCH v1 2/2] x86: kconfig: remove X86_UP_APIC [not found] ` <55015F690200007800068D86@suse.com> @ 2015-03-12 15:18 ` Luis R. Rodriguez 0 siblings, 0 replies; 9+ messages in thread From: Luis R. Rodriguez @ 2015-03-12 15:18 UTC (permalink / raw) To: Jan Beulich Cc: Luis R. Rodriguez, Juergen Gross, luto, thomas.petazzoni, andy.shevchenko, bhelgaas, rientjes, Ingo Molnar, x86, tglx, pure.logic, mingo, bp, linux-kernel, linux-pci, hpa On Thu, Mar 12, 2015 at 02:42:01AM -0600, Jan Beulich wrote: > >>> On 12.03.15 at 00:10, <mcgrof@do-not-panic.com> wrote: > > config X86_LOCAL_APIC > > def_bool y > > - depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || PCI_MSI > > + depends on X86_64 || SMP || X86_32_NON_STANDARD || PCI_MSI > > I.e. building a 32-bit kernel with APIC support but with !SMP, !PCI_MSI, > and !X86_32_NON_STANDARD will now be impossible. These are the requirements for X86_UP_APIC though. > Surely not what the patch description says. The only removal I see here is the option to opt-in or out of X86_UP_APIC when PCI_MSI is *not* enabled provided you meet the requirements. > > --- a/drivers/pci/Kconfig > > +++ b/drivers/pci/Kconfig > > @@ -5,6 +5,8 @@ config PCI_MSI > > bool "Message Signaled Interrupts (MSI and MSI-X)" > > depends on PCI > > select GENERIC_MSI_IRQ > > + select X86_LOCAL_APIC > > + select X86_IO_APIC > > I don't see the need for the latter - MSI specifically works without > any IO-APIC interaction. Although it can some MSI platforms need it as Bryan originally had expressed in his original patch [0] that Intel CE, Intel MID and Intel Quark are all 32-bit uniprocessor systems with IO-APICs that also use PCI-MSI, so this is not a hard requirement but rather compromising on enabling it since the the X86_IOAPIC cost is only ~12 KiB at with no performance penalty. https://lkml.org/lkml/2015/1/22/718 > And for the former you should decide > which way you want it - PCI_MSI select X86_LOCAL_APIC > (probably the right thing in x86, but surely wrong everwhere > else, i.e. this at least needs a condition tagged onto it) selects are done *iff* the dependencies are met, otherwise we can't select it, that's all, so the *iff* is implicit. > or > X86_LOCAL_APIC depend on PCI_MSI; in no case should this > be a forward _and_ reverse dependency. There is no reverse requirement here because of the select. The trick here is the select implies the depends the value you are selecting has. mcgrof@ergon ~/linux-next (git::master)$ export ARCH=alpha mcgrof@ergon ~/linux-next (git::master)$ make allnoconfig mcgrof@ergon ~/linux-next (git::master)$ make menuconfig --> System setup ---> [*] Message Signaled Interrupts (MSI and MSI-X) mcgrof@ergon ~/linux-next (git::master)$ grep -i APIC .config mcgrof@ergon ~/linux-next (git::master)$ echo $? 1 Luis ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-03-12 15:35 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-11 23:10 [PATCH v1 0/2] x86: simplify UP APIC conditions Luis R. Rodriguez
2015-03-11 23:10 ` [PATCH v1 1/2] x86: kconfig: remove X86_UP_IOAPIC Luis R. Rodriguez
2015-03-12 1:19 ` Bryan O'Donoghue
2015-03-12 15:35 ` Luis R. Rodriguez
2015-03-12 5:36 ` David Rientjes
2015-03-12 15:26 ` Luis R. Rodriguez
2015-03-11 23:10 ` [PATCH v1 2/2] x86: kconfig: remove X86_UP_APIC Luis R. Rodriguez
2015-03-12 8:42 ` Jan Beulich
[not found] ` <55015F690200007800068D86@suse.com>
2015-03-12 15:18 ` Luis R. Rodriguez
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).