[PATCH] x86_64: make amd quad core 8 socket system not be clustered_box - Kernel
This is a discussion on [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box - Kernel ; On Sun, Feb 24, 2008 at 01:29:03PM +0100, Andi Kleen wrote:
>On Sat, Feb 23, 2008 at 09:48:42PM -0800, Yinghai Lu wrote:
>>
>> quad core 8 socket system will have apic id lifting.the apic id range could
>> be ...
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Sun, Feb 24, 2008 at 01:29:03PM +0100, Andi Kleen wrote:
>On Sat, Feb 23, 2008 at 09:48:42PM -0800, Yinghai Lu wrote:
>>
>> quad core 8 socket system will have apic id lifting.the apic id range could
>> be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters
>> and that is large than 2. So it is treated as clustered_box.
>
>Ok I see you chose the quick hack over doing it properly ...
>
>>
>> and will get
>>
>> Marking TSC unstable due to TSCs unsynchronized
>>
>> even the CPUs have X86_FEATURE_CONSTANT_TSC set.
>
>I doubt that will do the right thing on AMD based vSMP,
>which also required the cluster check on AMD iirc.
>
Andi, Yes. AMD based vSMPowered systems uses clustered APICs, and this
check base on cpu vendor id is not good
.
Thanks,
Kiran
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Mon, Feb 25, 2008 at 11:08 AM, Ravikiran Thirumalai
wrote:
> On Sun, Feb 24, 2008 at 01:29:03PM +0100, Andi Kleen wrote:
> >On Sat, Feb 23, 2008 at 09:48:42PM -0800, Yinghai Lu wrote:
> >>
> >> quad core 8 socket system will have apic id lifting.the apic id range could
> >> be [4, 0x23]. and apic_is_clustered_box will think that need to three clusters
> >> and that is large than 2. So it is treated as clustered_box.
> >
> >Ok I see you chose the quick hack over doing it properly ...
> >
> >>
> >> and will get
> >>
> >> Marking TSC unstable due to TSCs unsynchronized
> >>
> >> even the CPUs have X86_FEATURE_CONSTANT_TSC set.
> >
> >I doubt that will do the right thing on AMD based vSMP,
> >which also required the cluster check on AMD iirc.
> >
>
> Andi, Yes. AMD based vSMPowered systems uses clustered APICs, and this
> check base on cpu vendor id is not good
.
please check if you happy with
http://lkml.org/lkml/2008/2/24/273
Thanks
Yinghai Lu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Mon, Feb 25, 2008 at 02:05:45PM -0800, Yinghai Lu wrote:
>On Mon, Feb 25, 2008 at 11:08 AM, Ravikiran Thirumalai
> wrote:
>> ...
>> Andi, Yes. AMD based vSMPowered systems uses clustered APICs, and this
>> check base on cpu vendor id is not good
.
>
>please check if you happy with
>
>http://lkml.org/lkml/2008/2/24/273
>
I don't quite understand the purpose of the patch to begin with. Looking at
the current x86 git tree, apic_is_clustered_box is used only to determine if
tsc is synchronized on the platform. The AMD docs imply that TSC's are not
guaranteed to be synced across cores between nodes -- Opteron BKDG for
family 10h, Section 2.9.4:
Note: Timers associated with different CPU cores in the same processor
increment at the same rate. Timers associated with different CPU cores
in different processors increment at slightly different rates if (1) they
are located on different nodes and (2) CLKIN for these nodes is derived from
different, non-synchronized oscillator sources.
But that is not what the x86 tree does (with your patches) -- it looks for the
X86_FEATURE_CONSTANT_TSC at unsynchronized_tsc() and assumes a synchronized
clock. Huh!?? Am i missing something here? X86_FEATURE_CONSTANT_TSC
is set from CPUID Fn8000_0007 -- TscInvariant bit, which implies TSC is
not affected by change in PM states. This does not talk about whether CLKIN
for different packages are from synchronized/non synchronized oscillator
sources in the above quote.
Thanks,
Kiran
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
> I don't quite understand the purpose of the patch to begin with. Looking at
> the current x86 git tree, apic_is_clustered_box is used only to determine if
> tsc is synchronized on the platform. The AMD docs imply that TSC's are not
> guaranteed to be synced across cores between nodes -- Opteron BKDG for
> family 10h, Section 2.9.4:
After long discussions with AMD they determined the CPUID flag
for sync RDTSC will imply synchronization between nodes.
If you can't support that in your hardware you're supposed
to clear it.
-Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Tue, Feb 26, 2008 at 04:46:25AM +0100, Andi Kleen wrote:
>> I don't quite understand the purpose of the patch to begin with. Looking at
>> the current x86 git tree, apic_is_clustered_box is used only to determine if
>> tsc is synchronized on the platform. The AMD docs imply that TSC's are not
>> guaranteed to be synced across cores between nodes -- Opteron BKDG for
>> family 10h, Section 2.9.4:
>
>After long discussions with AMD they determined the CPUID flag
>for sync RDTSC will imply synchronization between nodes.
Ah!
>
>If you can't support that in your hardware you're supposed
>to clear it.
Hmm! How would a hardware vendor do that? That doesn't seem to be clear in
the BKDG. (Well, this is the problem with undocumented features
)
Thanks,
Kiran
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Mon, Feb 25, 2008 at 8:05 PM, Ravikiran Thirumalai wrote:
> On Tue, Feb 26, 2008 at 04:46:25AM +0100, Andi Kleen wrote:
> >> I don't quite understand the purpose of the patch to begin with. Looking at
> >> the current x86 git tree, apic_is_clustered_box is used only to determine if
> >> tsc is synchronized on the platform. The AMD docs imply that TSC's are not
> >> guaranteed to be synced across cores between nodes -- Opteron BKDG for
> >> family 10h, Section 2.9.4:
> >
> >After long discussions with AMD they determined the CPUID flag
> >for sync RDTSC will imply synchronization between nodes.
>
> Ah!
>
>
> >
> >If you can't support that in your hardware you're supposed
> >to clear it.
>
> Hmm! How would a hardware vendor do that? That doesn't seem to be clear in
> the BKDG. (Well, this is the problem with undocumented features
)
>
any good sign for APIC_clustered box? there is apicid between cpus
even all cpu are quadcore and fully populated?
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Mon, Feb 25, 2008 at 09:27:42PM -0800, Yinghai Lu wrote:
>On Mon, Feb 25, 2008 at 8:05 PM, Ravikiran Thirumalai wrote:
>> On Tue, Feb 26, 2008 at 04:46:25AM +0100, Andi Kleen wrote:
>>
>> >
>> >If you can't support that in your hardware you're supposed
>> >to clear it.
>>
>> Hmm! How would a hardware vendor do that? That doesn't seem to be clear in
>> the BKDG. (Well, this is the problem with undocumented features
)
>>
>any good sign for APIC_clustered box? there is apicid between cpus
>even all cpu are quadcore and fully populated?
I would suggest checking the SLIT distances -- On AMD boxes, if you have three
different distances between nodes, then that system would be multiboard,
and there is no way TSCs can be synced. On Intel boxes, if there are two
different distances between nodes, then this would be a multi board/multi
chassi box and TSCs won't be synced. This is a more generic solution and
should work on Summit/Unisys boxes as well. (I am ignoring Intel CSI for
now. It might need the same treatment as AMD)
Comments?
Kiran
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Tue, Feb 26, 2008 at 10:42 AM, Ravikiran Thirumalai
wrote:
> On Mon, Feb 25, 2008 at 09:27:42PM -0800, Yinghai Lu wrote:
> >On Mon, Feb 25, 2008 at 8:05 PM, Ravikiran Thirumalai wrote:
> >> On Tue, Feb 26, 2008 at 04:46:25AM +0100, Andi Kleen wrote:
> >>
> >> >
>
> >> >If you can't support that in your hardware you're supposed
> >> >to clear it.
> >>
> >> Hmm! How would a hardware vendor do that? That doesn't seem to be clear in
> >> the BKDG. (Well, this is the problem with undocumented features
)
> >>
> >any good sign for APIC_clustered box? there is apicid between cpus
> >even all cpu are quadcore and fully populated?
>
> I would suggest checking the SLIT distances -- On AMD boxes, if you have three
> different distances between nodes, then that system would be multiboard,
> and there is no way TSCs can be synced. On Intel boxes, if there are two
> different distances between nodes, then this would be a multi board/multi
> chassi box and TSCs won't be synced. This is a more generic solution and
> should work on Summit/Unisys boxes as well. (I am ignoring Intel CSI for
> now. It might need the same treatment as AMD)
1. if acpi=off ?
2. some system will be treated wrong.
my four sockets system
ACPI: SLIT: nodes = 4
10 13 13 16
13 10 16 13
13 16 10 13
16 13 13 10
my eight sockets system
ACPI: SLIT: nodes = 8
10 12 12 14 14 14 14 16
12 10 14 12 14 14 12 14
12 14 10 14 12 12 14 14
14 12 14 10 12 12 14 14
14 14 12 12 10 14 12 14
14 14 12 12 14 10 14 12
14 12 14 14 12 14 10 12
16 14 14 14 14 12 12 10
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Kconfig configuration restore bug [Was: x86: vSMP selection in config]
Hi Roman.
We discovered a situation where we could set a
choice value in menuconfig but later when we either was
running menuconfig or oldconfig the value were changed.
I have created a minimal config that exhibit the error.
It was created in a pure mechanical trial-and-error fashion.
First the minimal Kconfig file:
# x86 configuration
choice
prompt "Subarchitecture Type"
config X86_PC
bool "PC-compatible"
config X86_VOYAGER
bool "Voyager (NCR)"
config X86_VSMP
bool "Support for ScaleMP vSMP"
depends on PCI
endchoice
config PCI
bool "PCI support" if !X86_VISWS
depends on !X86_VOYAGER
default y
config USB_ARCH_HAS_HCD
bool
default PCI
config USB
bool "Support for Host-side USB"
depends on USB_ARCH_HAS_HCD
config USB_PHIDGET
bool "USB Phidgets drivers"
depends on USB
config USB_PHIDGETMOTORCONTROL
bool "USB PhidgetMotorControl support"
depends on USB_PHIDGET
Next the saved .config that is used:
#
# Automatically generated make config: don't edit
# Linux kernel version: KERNELVERSION
# Tue Feb 26 20:27:09 2008
#
# CONFIG_X86_PC is not set
# CONFIG_X86_VOYAGER is not set
CONFIG_X86_VSMP=y
CONFIG_PCI=y
CONFIG_USB_ARCH_HAS_HCD=y
CONFIG_USB=y
# CONFIG_USB_PHIDGET is not set
When we enter menuconfig or are running oldconfig then we can see
that CONFIG_X86_PC is set to 'y' and CONFIG_X86_VSMP is set to 'n'.
If I in menuconfig select VSMP this setting is saved but then
oldconfig kicks in and we loose the setting again.
If I delete any of the config variables in the sample above then
we no longer change the values and we keep the VSMP equals 'y'.
Can you please take a look at this.
Thanks,
Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86: vSMP selection in config
On Sun, Feb 24, 2008 at 10:43:49PM -0800, Yinghai Lu wrote:
>
> find out vSMP setting is going away in config after make oldconfig
>
> vSMP need to PARAVIRT and PCI.
> so move PARAVIRT out of if PARAVIRT_GUEST, and make vSMP select PCI instead of
> depends on PCI
>
> after patch vSMP could stick there.
I'm certain that we have hit a Kconfig bug / limitation here and
the patch below is just a workaround for that issue.
I tracked it down to a minimal case and hope Roman can provide
either an explanation or a fix.
IMO VSMP can wait until this is resolved properly and we should not
apply this patch.
Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>
>1. if acpi=off ?
Well that is not a realistic scenario for any multi chassi NUMA machine,
since the proximity information is very important and turning acpi off
deprives the OS of this information.
>2. some system will be treated wrong.
>my four sockets system
>ACPI: SLIT: nodes = 4
> 10 13 13 16
> 13 10 16 13
> 13 16 10 13
> 16 13 13 10
>my eight sockets system
This makes it difficult to use SLIT for multi chassi detection then
.
Did not know such SLIT tables existed on single board machines.
Thanks,
Kiran
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86: vSMP selection in config
On Tuesday 26 February 2008 12:05:12 pm Sam Ravnborg wrote:
> On Sun, Feb 24, 2008 at 10:43:49PM -0800, Yinghai Lu wrote:
> >
> > find out vSMP setting is going away in config after make oldconfig
> >
> > vSMP need to PARAVIRT and PCI.
> > so move PARAVIRT out of if PARAVIRT_GUEST, and make vSMP select PCI instead of
> > depends on PCI
> >
> > after patch vSMP could stick there.
>
> I'm certain that we have hit a Kconfig bug / limitation here and
> the patch below is just a workaround for that issue.
>
> I tracked it down to a minimal case and hope Roman can provide
> either an explanation or a fix.
>
> IMO VSMP can wait until this is resolved properly and we should not
> apply this patch.
also Kconfig seems ignore
if PARAVIRT_GUEST
endif
and
to use config PARAVIRT in that if block.
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
wrote:
> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
> >
> >1. if acpi=off ?
>
> Well that is not a realistic scenario for any multi chassi NUMA machine,
> since the proximity information is very important and turning acpi off
> deprives the OS of this information.
>
>
> >2. some system will be treated wrong.
> >my four sockets system
> >ACPI: SLIT: nodes = 4
> > 10 13 13 16
> > 13 10 16 13
> > 13 16 10 13
> > 16 13 13 10
> >my eight sockets system
>
> This makes it difficult to use SLIT for multi chassi detection then
.
> Did not know such SLIT tables existed on single board machines.
Yes
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
wrote:
> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
> >
> >1. if acpi=off ?
>
> Well that is not a realistic scenario for any multi chassi NUMA machine,
> since the proximity information is very important and turning acpi off
> deprives the OS of this information.
actually OS should detect the distance instead of rely on SLIT.
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Tue, Feb 26, 2008 at 01:10:57PM -0800, Yinghai Lu wrote:
>On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
> wrote:
>> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>> >
>> >1. if acpi=off ?
>>
>> Well that is not a realistic scenario for any multi chassi NUMA machine,
>> since the proximity information is very important and turning acpi off
>> deprives the OS of this information.
>
>actually OS should detect the distance instead of rely on SLIT.
How does it do that? ACPI SRAT is needed to determine this, and it wouldn't
work if acpi=off.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Tue, Feb 26, 2008 at 1:24 PM, Ravikiran Thirumalai wrote:
>
> On Tue, Feb 26, 2008 at 01:10:57PM -0800, Yinghai Lu wrote:
> >On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
> > wrote:
> >> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
> >> >
> >> >1. if acpi=off ?
> >>
> >> Well that is not a realistic scenario for any multi chassi NUMA machine,
> >> since the proximity information is very important and turning acpi off
> >> deprives the OS of this information.
> >
> >actually OS should detect the distance instead of rely on SLIT.
>
> How does it do that? ACPI SRAT is needed to determine this, and it wouldn't
> work if acpi=off.
srat only have apicid to node mapping, and ram to node mapping.
for amd64 system, when acpi=off
ram to node mapping can be retrieved from pci conf space
apicid to node mapping could be calculated too... with boot_cpu_id
....plus coreid bits.
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Tue, Feb 26, 2008 at 03:16:16PM -0800, Yinghai Lu wrote:
>On Tue, Feb 26, 2008 at 1:24 PM, Ravikiran Thirumalai wrote:
>>
>> On Tue, Feb 26, 2008 at 01:10:57PM -0800, Yinghai Lu wrote:
>> >On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
>> > wrote:
>> >> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
>> >> >
>> >> >1. if acpi=off ?
>> >>
>> >> Well that is not a realistic scenario for any multi chassi NUMA machine,
>> >> since the proximity information is very important and turning acpi off
>> >> deprives the OS of this information.
>> >
>> >actually OS should detect the distance instead of rely on SLIT.
>>
>> How does it do that? ACPI SRAT is needed to determine this, and it wouldn't
>> work if acpi=off.
>
>srat only have apicid to node mapping, and ram to node mapping.
Which is very important for performance.
>
>for amd64 system, when acpi=off
>ram to node mapping can be retrieved from pci conf space
I was referring to all types of numa systems, not just AMD. The pci config
space is amd specific. Multi chassi systems in particular would be
acpi SRAT based. Anyways.
Kiran
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: [PATCH] x86_64: make amd quad core 8 socket system not be clustered_box v2
On Tue, Feb 26, 2008 at 3:31 PM, Ravikiran Thirumalai wrote:
> On Tue, Feb 26, 2008 at 03:16:16PM -0800, Yinghai Lu wrote:
> >On Tue, Feb 26, 2008 at 1:24 PM, Ravikiran Thirumalai wrote:
> >>
> >> On Tue, Feb 26, 2008 at 01:10:57PM -0800, Yinghai Lu wrote:
> >> >On Tue, Feb 26, 2008 at 12:32 PM, Ravikiran Thirumalai
> >> > wrote:
> >> >> On Tue, Feb 26, 2008 at 11:00:58AM -0800, Yinghai Lu wrote:
> >> >> >
> >> >> >1. if acpi=off ?
> >> >>
> >> >> Well that is not a realistic scenario for any multi chassi NUMA machine,
> >> >> since the proximity information is very important and turning acpi off
> >> >> deprives the OS of this information.
> >> >
> >> >actually OS should detect the distance instead of rely on SLIT.
> >>
> >> How does it do that? ACPI SRAT is needed to determine this, and it wouldn't
> >> work if acpi=off.
> >
> >srat only have apicid to node mapping, and ram to node mapping.
>
> Which is very important for performance.
>
>
> >
> >for amd64 system, when acpi=off
> >ram to node mapping can be retrieved from pci conf space
>
> I was referring to all types of numa systems, not just AMD. The pci config
> space is amd specific. Multi chassi systems in particular would be
> acpi SRAT based. Anyways.
can you try x86.git#testing your vsmp to see if is_vsmp_box is working?
http://people.redhat.com/mingo/x86.git/README
YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
Re: Kconfig configuration restore bug [Was: x86: vSMP selection in config]
Hi,
On Tue, 26 Feb 2008, Sam Ravnborg wrote:
> choice
> prompt "Subarchitecture Type"
>
> config X86_PC
> bool "PC-compatible"
>
> config X86_VOYAGER
> bool "Voyager (NCR)"
>
> config X86_VSMP
> bool "Support for ScaleMP vSMP"
> depends on PCI
>
> endchoice
>
> config PCI
> bool "PCI support" if !X86_VISWS
> depends on !X86_VOYAGER
> default y
The basic problem is that this is a recursive dependency - PCI depends on
the choice and the choice depends on PCI. IMO X86_VSMP cannot depend on
PCI.
I'm looking into why this hasn't been picked up by the dependency check...
bye, Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
[PATCH 2/3] fix choice dependency check
Properly check the dependency of choices as a group.
Also fix that sym_check_deps() correctly terminates the dependency loop
error check (otherwise it would continue printing the dependency chain).
Signed-off-by: Roman Zippel
---
scripts/kconfig/symbol.c | 94 ++++++++++++++++++++++++++++++++++++++---------
1 file changed, 76 insertions(+), 18 deletions(-)
Index: linux-2.6/scripts/kconfig/symbol.c
================================================== =================
--- linux-2.6.orig/scripts/kconfig/symbol.c
+++ linux-2.6/scripts/kconfig/symbol.c
@@ -762,8 +762,6 @@ struct symbol **sym_re_search(const char
}
-struct symbol *sym_check_deps(struct symbol *sym);
-
static struct symbol *sym_check_expr_deps(struct expr *e)
{
struct symbol *sym;
@@ -795,40 +793,100 @@ static struct symbol *sym_check_expr_dep
}
/* return NULL when dependencies are OK */
-struct symbol *sym_check_deps(struct symbol *sym)
+static struct symbol *sym_check_sym_deps(struct symbol *sym)
{
struct symbol *sym2;
struct property *prop;
- if (sym->flags & SYMBOL_CHECK) {
- fprintf(stderr, "%s:%d:error: found recursive dependency: %s",
- sym->prop->file->name, sym->prop->lineno, sym->name);
- return sym;
- }
- if (sym->flags & SYMBOL_CHECKED)
- return NULL;
-
- sym->flags |= (SYMBOL_CHECK | SYMBOL_CHECKED);
sym2 = sym_check_expr_deps(sym->rev_dep.expr);
if (sym2)
- goto out;
+ return sym2;
for (prop = sym->prop; prop; prop = prop->next) {
if (prop->type == P_CHOICE || prop->type == P_SELECT)
continue;
sym2 = sym_check_expr_deps(prop->visible.expr);
if (sym2)
- goto out;
+ break;
if (prop->type != P_DEFAULT || sym_is_choice(sym))
continue;
sym2 = sym_check_expr_deps(prop->expr);
if (sym2)
- goto out;
+ break;
}
-out:
+
+ return sym2;
+}
+
+static struct symbol *sym_check_choice_deps(struct symbol *choice)
+{
+ struct symbol *sym, *sym2;
+ struct property *prop;
+ struct expr *e;
+
+ prop = sym_get_choice_prop(choice);
+ expr_list_for_each_sym(prop->expr, e, sym)
+ sym->flags |= (SYMBOL_CHECK | SYMBOL_CHECKED);
+
+ choice->flags |= (SYMBOL_CHECK | SYMBOL_CHECKED);
+ sym2 = sym_check_sym_deps(choice);
+ choice->flags &= ~SYMBOL_CHECK;
if (sym2)
- fprintf(stderr, " -> %s%s", sym->name, sym2 == sym? "\n": "");
- sym->flags &= ~SYMBOL_CHECK;
+ goto out;
+
+ expr_list_for_each_sym(prop->expr, e, sym) {
+ sym2 = sym_check_sym_deps(sym);
+ if (sym2) {
+ fprintf(stderr, " -> %s", sym->name);
+ break;
+ }
+ }
+out:
+ expr_list_for_each_sym(prop->expr, e, sym)
+ sym->flags &= ~SYMBOL_CHECK;
+
+ if (sym2 && sym_is_choice_value(sym2) &&
+ prop_get_symbol(sym_get_choice_prop(sym2)) == choice)
+ sym2 = choice;
+
+ return sym2;
+}
+
+struct symbol *sym_check_deps(struct symbol *sym)
+{
+ struct symbol *sym2;
+ struct property *prop;
+
+ if (sym->flags & SYMBOL_CHECK) {
+ fprintf(stderr, "%s:%d:error: found recursive dependency: %s",
+ sym->prop->file->name, sym->prop->lineno,
+ sym->name ? sym->name : "");
+ return sym;
+ }
+ if (sym->flags & SYMBOL_CHECKED)
+ return NULL;
+
+ if (sym_is_choice_value(sym)) {
+ /* for choice groups start the check with main choice symbol */
+ prop = sym_get_choice_prop(sym);
+ sym2 = sym_check_deps(prop_get_symbol(prop));
+ } else if (sym_is_choice(sym)) {
+ sym2 = sym_check_choice_deps(sym);
+ } else {
+ sym->flags |= (SYMBOL_CHECK | SYMBOL_CHECKED);
+ sym2 = sym_check_sym_deps(sym);
+ sym->flags &= ~SYMBOL_CHECK;
+ }
+
+ if (sym2) {
+ fprintf(stderr, " -> %s", sym->name ? sym->name : "");
+ if (sym2 == sym) {
+ fprintf(stderr, "\n");
+ zconfnerrs++;
+ sym2 = NULL;
+ }
+ }
+
return sym2;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/