[PATCH 0/4] x86: random clean-ups

[PATCH 0/4] x86: random clean-ups

[Dmitri Vorobiev]

Hi Ingo, Peter, Thomas,

The following patches contain a few clean-ups, mostly
stemming from analyzis of sparse messages.

The patches are against current x86.git.

Please consider.

Thanks,
Dmitri Vorobiev

[PATCH 1/4] x86: remove unused function amd_init_cpu()

[Dmitri Vorobiev]

There are no users for the function amd_init_cpu() defined in
arch/x86/kernel/cpu/amd.c. This patch removes this routine.

This patch was build-tested using defconfigs for i386 and x86_64,
and a few randconfig instances. Runtime tests were performed by
booting 32- and 64-bit x86 boxen up to the shell prompt.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
---
 arch/x86/kernel/cpu/amd.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 0173065..2458668 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -343,10 +343,4 @@ static struct cpu_dev amd_cpu_dev __cpuinitdata = {
 	.c_size_cache	= amd_size_cache,
 };
 
-int __init amd_init_cpu(void)
-{
-	cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;
-	return 0;
-}
-
 cpu_vendor_dev_register(X86_VENDOR_AMD, &amd_cpu_dev);
-- 
1.5.3

[PATCH 2/4] x86: remove unused code and data in nexgen.c

[Dmitri Vorobiev]

Nobody calls the function nexgen_init_cpu(), therefore it
can be removed. The variable nexgen_cpu_dev is used only by
this function, so this patch removes useless data as well.

This patch was build-tested using defconfigs for i386 and x86_64.
It also survived a few randconfig instances for i386. Runtime
tests were performed by booting 32-bit and 64-bit x86 boxen up to
the shell prompt.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
---
 arch/x86/kernel/cpu/nexgen.c |   19 -------------------
 1 files changed, 0 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kernel/cpu/nexgen.c b/arch/x86/kernel/cpu/nexgen.c
index 5d5e1c1..f5753b4 100644
--- a/arch/x86/kernel/cpu/nexgen.c
+++ b/arch/x86/kernel/cpu/nexgen.c
@@ -38,22 +38,3 @@ static void __cpuinit nexgen_identify(struct cpuinfo_x86 *c)
 	if (deep_magic_nexgen_probe())
 		strcpy(c->x86_vendor_id, "NexGenDriven");
 }
-
-static struct cpu_dev nexgen_cpu_dev __cpuinitdata = {
-	.c_vendor	= "Nexgen",
-	.c_ident	= { "NexGenDriven" },
-	.c_models = {
-			{ .vendor = X86_VENDOR_NEXGEN,
-			  .family = 5,
-			  .model_names = { [1] = "Nx586" }
-			},
-	},
-	.c_init		= init_nexgen,
-	.c_identify	= nexgen_identify,
-};
-
-int __init nexgen_init_cpu(void)
-{
-	cpu_devs[X86_VENDOR_NEXGEN] = &nexgen_cpu_dev;
-	return 0;
-}
-- 
1.5.3

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[H. Peter Anvin]

Dmitri Vorobiev wrote:
> Nobody calls the function nexgen_init_cpu(), therefore it
> can be removed. The variable nexgen_cpu_dev is used only by
> this function, so this patch removes useless data as well.
> 
> This patch was build-tested using defconfigs for i386 and x86_64.
> It also survived a few randconfig instances for i386. Runtime
> tests were performed by booting 32-bit and 64-bit x86 boxen up to
> the shell prompt.

NAK.  This is broken.

The reason nexgen_cpu_dev is unreferenced is because the file is missing 
a cpu_vendor_dev_register(), which should be added.

	-hpa

[PATCH 2/2 v2] x86: remove unused function, register the cpu vendor in nexgen.c

[Dmitri Vorobiev]

Nobody calls the function nexgen_init_cpu(), which is defined
in arch/x86/kernel/cpu/nexgen.c. Therefore this routine can
be removed, and this patch does so.

Additionally, this patch adds the missing registration of the
NexGen CPUs by the cpu_vendor_dev_register() macro.

This patch was build-tested using the defconfig for i386. It
also survived a few randconfig instances. Runtime tests were
performed by booting an i386 box up to the shell prompt.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
---
Peter,

Thanks for your review. Hopefully this version is OK.

 arch/x86/kernel/cpu/nexgen.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/nexgen.c b/arch/x86/kernel/cpu/nexgen.c
index 5d5e1c1..3117e96 100644
--- a/arch/x86/kernel/cpu/nexgen.c
+++ b/arch/x86/kernel/cpu/nexgen.c
@@ -52,8 +52,4 @@ static struct cpu_dev nexgen_cpu_dev __cpuinitdata = {
 	.c_identify	= nexgen_identify,
 };
 
-int __init nexgen_init_cpu(void)
-{
-	cpu_devs[X86_VENDOR_NEXGEN] = &nexgen_cpu_dev;
-	return 0;
-}
+cpu_vendor_dev_register(X86_VENDOR_NEXGEN, &nexgen_cpu_dev);
-- 
1.5.3

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[Andi Kleen]

"H. Peter Anvin" <hpa@zytor.com> writes:

> Dmitri Vorobiev wrote:
>> Nobody calls the function nexgen_init_cpu(), therefore it
>> can be removed. The variable nexgen_cpu_dev is used only by
>> this function, so this patch removes useless data as well.
>> This patch was build-tested using defconfigs for i386 and x86_64.
>> It also survived a few randconfig instances for i386. Runtime
>> tests were performed by booting 32-bit and 64-bit x86 boxen up to
>> the shell prompt.
>
> NAK.  This is broken.
>
> The reason nexgen_cpu_dev is unreferenced is because the file is
> missing a cpu_vendor_dev_register(), which should be added.

If we did that long without it wouldn't it be better to just remove it?

Also NexGen was never shipped anyways, but bought by AMD and became
the K6. So I assume whatever this file is supposed to do is done
by amd.c anyways.

-Andi

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[Dmitri Vorobiev]

Andi Kleen пишет:
> "H. Peter Anvin" <hpa@zytor.com> writes:
> 
>> Dmitri Vorobiev wrote:
>>> Nobody calls the function nexgen_init_cpu(), therefore it
>>> can be removed. The variable nexgen_cpu_dev is used only by
>>> this function, so this patch removes useless data as well.
>>> This patch was build-tested using defconfigs for i386 and x86_64.
>>> It also survived a few randconfig instances for i386. Runtime
>>> tests were performed by booting 32-bit and 64-bit x86 boxen up to
>>> the shell prompt.
>> NAK.  This is broken.
>>
>> The reason nexgen_cpu_dev is unreferenced is because the file is
>> missing a cpu_vendor_dev_register(), which should be added.
> 
> If we did that long without it wouldn't it be better to just remove it?
> 
> Also NexGen was never shipped anyways, but bought by AMD and became
> the K6. So I assume whatever this file is supposed to do is done
> by amd.c anyways.
> 

IOW, you claim that now it's safe enough to throw away the NexGen
altogether?

Dmitri

> -Andi
> 

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[H. Peter Anvin]

Dmitri Vorobiev wrote:
>>
>> Also NexGen was never shipped anyways, but bought by AMD and became
>> the K6. So I assume whatever this file is supposed to do is done
>> by amd.c anyways.
> 
> IOW, you claim that now it's safe enough to throw away the NexGen
> altogether?
> 

If it never shipped, then yes, it should be safe to remove.  We don't 
support other vendors that never went to product, e.g. RISE or Montalvo.

	-hpa

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[Dmitri Vorobiev]

H. Peter Anvin пишет:
> Dmitri Vorobiev wrote:
>>>
>>> Also NexGen was never shipped anyways, but bought by AMD and became
>>> the K6. So I assume whatever this file is supposed to do is done
>>> by amd.c anyways.
>>
>> IOW, you claim that now it's safe enough to throw away the NexGen
>> altogether?
>>
> 
> If it never shipped, then yes, it should be safe to remove.  We don't
> support other vendors that never went to product, e.g. RISE or Montalvo.

OK, then please drop this patch from the series (the other changes are OK,
I presume), and I'll try to take care of NexGen removal with a separate patch.

Thanks,
Dmitri

> 
>     -hpa
> 

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[H. Peter Anvin]

Dmitri Vorobiev wrote:
> 
> OK, then please drop this patch from the series (the other changes are OK,
> I presume), and I'll try to take care of NexGen removal with a separate patch.
> 

Acked-by: H. Peter Anvin <hpa@zytor.com>

... on the rest.

	-hpa

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[Andi Kleen]

H. Peter Anvin wrote:
> Dmitri Vorobiev wrote:
>>>
>>> Also NexGen was never shipped anyways, but bought by AMD and became
>>> the K6. So I assume whatever this file is supposed to do is done
>>> by amd.c anyways.
>>
>> IOW, you claim that now it's safe enough to throw away the NexGen
>> altogether?
>>
> 
> If it never shipped, then yes, it should be safe to remove. 

It was shipped as AMD K6, but I have no idea if this file is supposed
to handle real k6s or just early nexgen engineering samples.

But since it has been basically broken forever (or at least for 
all of 2.6) it is probably safe to remove until someone complains.

-Andi

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[H. Peter Anvin]

Andi Kleen wrote:
> 
> It was shipped as AMD K6, but I have no idea if this file is supposed
> to handle real k6s or just early nexgen engineering samples.
> 
> But since it has been basically broken forever (or at least for 
> all of 2.6) it is probably safe to remove until someone complains.
> 

The file only triggers on the "NexGenDriven" CPU signature.  Let's kill it.

	-hpa

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[Alan Cox]

On Sun, 20 Apr 2008 15:34:53 -0400
"H. Peter Anvin" <hpa@zytor.com> wrote:

> Dmitri Vorobiev wrote:
> >>
> >> Also NexGen was never shipped anyways, but bought by AMD and became
> >> the K6. So I assume whatever this file is supposed to do is done
> >> by amd.c anyways.
> > 
> > IOW, you claim that now it's safe enough to throw away the NexGen
> > altogether?
> > 
> 
> If it never shipped, then yes, it should be safe to remove.  We don't 
> support other vendors that never went to product, e.g. RISE or Montalvo.

There are/were a few Nexgen boxes out there - strange beast where the
BIOS code emulated some instructions. So few I doubt killing it off is a
problem at all.

Alan

Re: [PATCH 2/4] x86: remove unused code and data in nexgen.c

[Dmitri Vorobiev]

Alan Cox пишет:
> On Sun, 20 Apr 2008 15:34:53 -0400
> "H. Peter Anvin" <hpa@zytor.com> wrote:
> 
>> Dmitri Vorobiev wrote:
>>>> Also NexGen was never shipped anyways, but bought by AMD and became
>>>> the K6. So I assume whatever this file is supposed to do is done
>>>> by amd.c anyways.
>>> IOW, you claim that now it's safe enough to throw away the NexGen
>>> altogether?
>>>
>> If it never shipped, then yes, it should be safe to remove.  We don't 
>> support other vendors that never went to product, e.g. RISE or Montalvo.
> 
> There are/were a few Nexgen boxes out there - strange beast where the
> BIOS code emulated some instructions. So few I doubt killing it off is a
> problem at all.

The patch that kills the NexGen is already in Ingo's x86 tree, therefore
the beast is dead, at least from the kernel viewpoint.

Thanks,
Dmitri

> 
> Alan
> 

[PATCH 3/4] x86_64: the standard_io_resources[] array can become static

[Dmitri Vorobiev]

In arch/x86/kernel/setup_64.c, the standard_io_resources array
is needlessly defined as global. This patch makes this variable
static.

This patch was successfully build-tested using the defconfig
for x86_64. Runtime test was performed by booting a 64-bit x86
box up to the shell prompt.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
---
 arch/x86/kernel/setup_64.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index cd245ef..0332ae2 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -117,7 +117,7 @@ extern int root_mountflags;
 
 char __initdata command_line[COMMAND_LINE_SIZE];
 
-struct resource standard_io_resources[] = {
+static struct resource standard_io_resources[] = {
 	{ .name = "dma1", .start = 0x00, .end = 0x1f,
 		.flags = IORESOURCE_BUSY | IORESOURCE_IO },
 	{ .name = "pic1", .start = 0x20, .end = 0x21,
-- 
1.5.3

[PATCH 4/4] x86: conditionally compile sysfs stuff in intel_cacheinfo.c

[Dmitri Vorobiev]

Four functions defined in arch/x86/kernel/cpu/intel_cacheinfo.c:

cache_shared_cpu_map_setup()
cache_remove_shared_cpu_map()
free_cache_attributes()
detect_cache_attributes()

are needed only in the code that is conditionally compiled when
CONFIG_SYSFS is defined. The same is true about the cpuid4_info[]
array. This patch places the definitions of these functions and
the array inside the corresponding ifdef.

This patch was build-tested both with sysfs turned off and on.
The kernel was built successfully in both cases. Runtime tests
were performed using x86 and x86_64 machines, which booted to
the shell prompt successfully.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
---
 arch/x86/kernel/cpu/intel_cacheinfo.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c
index 1b88986..d743dfe 100644
--- a/arch/x86/kernel/cpu/intel_cacheinfo.c
+++ b/arch/x86/kernel/cpu/intel_cacheinfo.c
@@ -450,6 +450,8 @@ unsigned int __cpuinit init_intel_cacheinfo(struct cpuinfo_x86 *c)
 	return l2;
 }
 
+#ifdef CONFIG_SYSFS
+
 /* pointer to _cpuid4_info array (for each cache leaf) */
 static struct _cpuid4_info *cpuid4_info[NR_CPUS];
 #define CPUID4_INFO_IDX(x,y)    (&((cpuid4_info[x])[y]))
@@ -553,8 +555,6 @@ out:
 	return retval;
 }
 
-#ifdef CONFIG_SYSFS
-
 #include <linux/kobject.h>
 #include <linux/sysfs.h>
 
-- 
1.5.3

Re: [PATCH 0/4] x86: random clean-ups

[Ingo Molnar]

* Dmitri Vorobiev <dmitri.vorobiev@gmail.com> wrote:

> Hi Ingo, Peter, Thomas,
> 
> The following patches contain a few clean-ups, mostly stemming from 
> analyzis of sparse messages.
> 
> The patches are against current x86.git.

thanks Dmitri, applied #1/#2/#4, as per the discussion.

	Ingo

Re: [PATCH 0/4] x86: random clean-ups

[Ingo Molnar]

* Dmitri Vorobiev <dmitri.vorobiev@gmail.com> wrote:

> > thanks Dmitri, applied #1/#2/#4, as per the discussion.
> 
> Hi Ingo,
> 
> This should be #1, #3 and #4. The NexGen patch, which was #2 in the 
> original series, have been replaced by a bigger patch that kills 
> NexGen altogether. Plese check, if you have time for that.

yeah, i did the right thing and described the wrong selection :) You can 
check the end result via:

  http://people.redhat.com/mingo/x86.git/README

	Ingo

Re: [PATCH 0/4] x86: random clean-ups

[Dmitri Vorobiev]

Ingo Molnar пишет:
> * Dmitri Vorobiev <dmitri.vorobiev@gmail.com> wrote:
> 
>>> thanks Dmitri, applied #1/#2/#4, as per the discussion.
>> Hi Ingo,
>>
>> This should be #1, #3 and #4. The NexGen patch, which was #2 in the 
>> original series, have been replaced by a bigger patch that kills 
>> NexGen altogether. Plese check, if you have time for that.
> 
> yeah, i did the right thing and described the wrong selection :) You can 
> check the end result via:
> 
>   http://people.redhat.com/mingo/x86.git/README

... which indicates that #4 is still not there, which is a pity, since
the #4 patch gave me a more-than-negligible binary shrinkage for the case
CONFIG_SYSFS=n :)

Dmitri

> 
> 	Ingo
> 

Re: [PATCH 0/4] x86: random clean-ups

[Ingo Molnar]

* Dmitri Vorobiev <dmitri.vorobiev@gmail.com> wrote:

> >> This should be #1, #3 and #4. The NexGen patch, which was #2 in the 
> >> original series, have been replaced by a bigger patch that kills 
> >> NexGen altogether. Plese check, if you have time for that.
> > 
> > yeah, i did the right thing and described the wrong selection :) You 
> > can check the end result via:
> > 
> >   http://people.redhat.com/mingo/x86.git/README
> 
> ... which indicates that #4 is still not there, which is a pity, since 
> the #4 patch gave me a more-than-negligible binary shrinkage for the 
> case CONFIG_SYSFS=n :)

oh, yes, i forgot - that collided with some changes to that file - 
percpu changes and 4Kcpu support. I think we should delay #4 to a bit 
later, when the other changes have gone in - or you could try to send a 
patch against sched-devel.git/latest which should show the merged up 
status of that code:

  http://people.redhat.com/mingo/sched-devel.git/README

sched-devel/latest has all of the x86 tree 'embedded' in it - plus the 
4Kcpu changes as well.

	Ingo

Re: [PATCH 0/4] x86: random clean-ups

[Dmitri Vorobiev]

Ingo Molnar пишет:
> * Dmitri Vorobiev <dmitri.vorobiev@gmail.com> wrote:
> 
>>>> This should be #1, #3 and #4. The NexGen patch, which was #2 in the 
>>>> original series, have been replaced by a bigger patch that kills 
>>>> NexGen altogether. Plese check, if you have time for that.
>>> yeah, i did the right thing and described the wrong selection :) You 
>>> can check the end result via:
>>>
>>>   http://people.redhat.com/mingo/x86.git/README
>> ... which indicates that #4 is still not there, which is a pity, since 
>> the #4 patch gave me a more-than-negligible binary shrinkage for the 
>> case CONFIG_SYSFS=n :)
> 
> oh, yes, i forgot - that collided with some changes to that file - 
> percpu changes and 4Kcpu support. I think we should delay #4 to a bit 
> later, when the other changes have gone in - or you could try to send a 
> patch against sched-devel.git/latest which should show the merged up 
> status of that code:
> 
>   http://people.redhat.com/mingo/sched-devel.git/README
> 
> sched-devel/latest has all of the x86 tree 'embedded' in it - plus the 
> 4Kcpu changes as well.

OK, let's wait till the things calm down a bit. The "sysfs disabled" case
should be rare enough to not be in a big rush here.

I'll resubmit this after the merge window has closed.

Thanks,
Dmitri

> 
> 	Ingo
>