[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <86802c440803210217p2793195bj989be0b25017414@mail.gmail.com>
Date: Fri, 21 Mar 2008 02:17:57 -0700
From: "Yinghai Lu" <yhlu.kernel@...il.com>
To: "Ingo Molnar" <mingo@...e.hu>
Cc: "Ravikiran G Thirumalai" <kiran@...lex86.org>,
"Andrew Morton" <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org,
"Glauber de Oliveira Costa" <gcosta@...hat.com>,
"Andi Kleen" <ak@...e.de>, shai@...lex86.org
Subject: Re: [patch 1/4] x86: vSMP: Fix is_vsmp_box()
On Fri, Mar 21, 2008 at 2:11 AM, Ingo Molnar <mingo@...e.hu> wrote:
>
> * Ravikiran G Thirumalai <kiran@...lex86.org> wrote:
>
>
> > >> /* Check if we are running on a ScaleMP vSMP box */
> > >> - if (read_pci_config(0, 0x1f, 0, PCI_VENDOR_ID) ==
> > >> - (PCI_VENDOR_ID_SCALEMP || (PCI_DEVICE_ID_SCALEMP_VSMP_CTL << 16)))
> > >> + if ((read_pci_config_16(0, 0x1f, 0, PCI_VENDOR_ID) ==
> > >> + PCI_VENDOR_ID_SCALEMP) &&
> > >> + (read_pci_config_16(0, 0x1f, 0, PCI_DEVICE_ID) ==
> > >> + PCI_DEVICE_ID_SCALEMP_VSMP_CTL))
> > >> vsmp = 1;
> > >>
> > >> return vsmp;
> > >
> > >why read two times
> > >
> >
> > Well, the pci cfg space read happens just _once_ during the boot, as
> > the result is cached in a static flag. The above code is better
> > readable. So readability is better than micro-optimization here.
>
> i think the patch below results in even more readable and a bit smaller
> code - because it's such a simple check?
>
> OTOH, you are right in general, for example in mmconf-fam10h_64.c's
> get_fam10h_pci_mmconf_base() function, we do this:
>
> id = read_pci_config(bus, slot, 0, PCI_VENDOR_ID);
>
> vendor = id & 0xffff;
> device = (id>>16) & 0xffff;
> if (pci_probes[i].vendor == vendor &&
> pci_probes[i].device == device) {
>
> here it would indeed be cleaner to simply do:
>
> vendor = read_pci_config_16(bus, slot, 0, PCI_VENDOR_ID);
> device = read_pci_config_16(bus, slot, 0, PCI_DEVICE_ID);
>
> instead of open-coding the 16-bit splitting.
>
> Ingo
>
> ------------>
> Subject: x86: vsmp fix x86 vsmp fix is vsmp box cleanup
> From: Ingo Molnar <mingo@...e.hu>
> Date: Fri Mar 21 09:55:06 CET 2008
>
> code got a bit smaller:
>
> arch/x86/kernel/vsmp_64.o:
>
> text data bss dec hex filename
> 205 4 0 209 d1 vsmp_64.o.before
> 181 4 0 185 b9 vsmp_64.o.after
good evidence.
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists