Re: [patch 1/4] x86: vSMP: Fix is_vsmp_box()

[Yinghai Lu]

On Fri, Mar 21, 2008 at 2:11 AM, Ingo Molnar <mingo@xxxxxxx> wrote:
>
>  * Ravikiran G Thirumalai <kiran@xxxxxxxxxxxx> 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@xxxxxxx>
>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/