* [PATCH -tip] x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static
@ 2009-04-11 7:25 Jaswinder Singh Rajput
2009-04-12 10:39 ` [tip:x86/cleanups] " Jaswinder Singh Rajput
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Jaswinder Singh Rajput @ 2009-04-11 7:25 UTC (permalink / raw)
To: Ingo Molnar, x86 maintainers, Suresh Siddha, LKML
Impact: reduce kernel size a bit, avoid sparse warning
Fixes sparse warning:
arch/x86/kernel/apic/x2apic_cluster.c:13:1: warning: symbol 'per_cpu__x86_cpu_to_logical_apicid' was not declared. Should it be static?
Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
---
arch/x86/kernel/apic/x2apic_cluster.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 4a903e2..8e4cbb2 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -10,7 +10,7 @@
#include <asm/apic.h>
#include <asm/ipi.h>
-DEFINE_PER_CPU(u32, x86_cpu_to_logical_apicid);
+static DEFINE_PER_CPU(u32, x86_cpu_to_logical_apicid);
static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
{
--
1.6.0.6
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:x86/cleanups] x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static
2009-04-11 7:25 [PATCH -tip] x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static Jaswinder Singh Rajput
@ 2009-04-12 10:39 ` Jaswinder Singh Rajput
2009-04-12 10:51 ` [PATCH -tip] " Ingo Molnar
2009-04-12 11:21 ` [tip:x86/cleanups] " Jaswinder Singh Rajput
2 siblings, 0 replies; 5+ messages in thread
From: Jaswinder Singh Rajput @ 2009-04-12 10:39 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, jaswinder, jaswinderrajput,
suresh.b.siddha, tglx, mingo
Commit-ID: 0be88dbc90fceb2261a8358b796d203475235da9
Gitweb: http://git.kernel.org/tip/0be88dbc90fceb2261a8358b796d203475235da9
Author: Jaswinder Singh Rajput <jaswinder@kernel.org>
AuthorDate: Sat, 11 Apr 2009 12:55:26 +0530
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 12 Apr 2009 12:38:01 +0200
x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static
Impact: reduce kernel size a bit, address sparse warning
Fixes sparse warning:
arch/x86/kernel/apic/x2apic_cluster.c:13:1: warning: symbol 'per_cpu__x86_cpu_to_logical_apicid' was not declared. Should it be static?
Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
LKML-Reference: <1239434726.4418.24.camel@localhost.localdomain>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/x86/kernel/apic/x2apic_cluster.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 4a903e2..8e4cbb2 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -10,7 +10,7 @@
#include <asm/apic.h>
#include <asm/ipi.h>
-DEFINE_PER_CPU(u32, x86_cpu_to_logical_apicid);
+static DEFINE_PER_CPU(u32, x86_cpu_to_logical_apicid);
static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
{
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH -tip] x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static
2009-04-11 7:25 [PATCH -tip] x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static Jaswinder Singh Rajput
2009-04-12 10:39 ` [tip:x86/cleanups] " Jaswinder Singh Rajput
@ 2009-04-12 10:51 ` Ingo Molnar
2009-04-12 11:47 ` Jaswinder Singh Rajput
2009-04-12 11:21 ` [tip:x86/cleanups] " Jaswinder Singh Rajput
2 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2009-04-12 10:51 UTC (permalink / raw)
To: Jaswinder Singh Rajput; +Cc: x86 maintainers, Suresh Siddha, LKML, Sam Ravnborg
* Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> Impact: reduce kernel size a bit, avoid sparse warning
>
> Fixes sparse warning:
> arch/x86/kernel/apic/x2apic_cluster.c:13:1: warning: symbol 'per_cpu__x86_cpu_to_logical_apicid' was not declared. Should it be static?
>
> Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> ---
> arch/x86/kernel/apic/x2apic_cluster.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Applied, thanks.
There is a not so small nit:
> Impact: reduce kernel size a bit, avoid sparse warning
>
> Fixes sparse warning:
the thing is, we dont 'fix', nor do we 'avoid' Sparse warnings!
We _read_ them, _understand_ them, and then we act upon them, fixing
the problem they expose.
Or, if there is no problem exposed, we annotate the code to fix the
Sparse false positive warning.
Your changelog does not tell us anything whether you went through
that thought process. I had to double-check it and had to create
this information from scratch.
Please take this as a last warning: you send lots of patches that
address various things mechanically, often without thinking through
the effects. They are expensive to maintain, because they cause
churn and because people often have to do more work accepting them
than you did creating them!
You sent a hundred patches in two weeks and they are not applied yet
- and this is why: it is expensive to filter through them and if you
dont do it we can only do it by simply not taking them all that
easily. Taking them simply does not scale.
And if you write a hundred patches in two weeks you _really_ have to
ask yourself whether your quality controls are strong enough before
emitting them. There are highly productive members of the Linux
community who only send a dozen patches per _year_.
You really want to avoid the 'also sends clueless patches'
contributor label, because the only way to deal with such patches as
a maintainer is to rate-throttle them.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tip:x86/cleanups] x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static
2009-04-11 7:25 [PATCH -tip] x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static Jaswinder Singh Rajput
2009-04-12 10:39 ` [tip:x86/cleanups] " Jaswinder Singh Rajput
2009-04-12 10:51 ` [PATCH -tip] " Ingo Molnar
@ 2009-04-12 11:21 ` Jaswinder Singh Rajput
2 siblings, 0 replies; 5+ messages in thread
From: Jaswinder Singh Rajput @ 2009-04-12 11:21 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, jaswinder, jaswinderrajput,
suresh.b.siddha, tglx, mingo
Commit-ID: 2de1f33e99cec5fd79542a1d0e26efb9c36a98bb
Gitweb: http://git.kernel.org/tip/2de1f33e99cec5fd79542a1d0e26efb9c36a98bb
Author: Jaswinder Singh Rajput <jaswinder@kernel.org>
AuthorDate: Sat, 11 Apr 2009 12:55:26 +0530
Committer: Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 12 Apr 2009 12:39:24 +0200
x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static
Impact: reduce kernel size a bit, address sparse warning
Addresses the problem pointed out by this sparse warning:
arch/x86/kernel/apic/x2apic_cluster.c:13:1: warning: symbol 'per_cpu__x86_cpu_to_logical_apicid' was not declared. Should it be static?
Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
Cc: Suresh Siddha <suresh.b.siddha@intel.com>
LKML-Reference: <1239434726.4418.24.camel@localhost.localdomain>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
arch/x86/kernel/apic/x2apic_cluster.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 4a903e2..8e4cbb2 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -10,7 +10,7 @@
#include <asm/apic.h>
#include <asm/ipi.h>
-DEFINE_PER_CPU(u32, x86_cpu_to_logical_apicid);
+static DEFINE_PER_CPU(u32, x86_cpu_to_logical_apicid);
static int x2apic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
{
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH -tip] x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static
2009-04-12 10:51 ` [PATCH -tip] " Ingo Molnar
@ 2009-04-12 11:47 ` Jaswinder Singh Rajput
0 siblings, 0 replies; 5+ messages in thread
From: Jaswinder Singh Rajput @ 2009-04-12 11:47 UTC (permalink / raw)
To: Ingo Molnar; +Cc: x86 maintainers, Suresh Siddha, LKML, Sam Ravnborg
Hello Ingo,
On Sun, 2009-04-12 at 12:51 +0200, Ingo Molnar wrote:
> * Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
>
> > Impact: reduce kernel size a bit, avoid sparse warning
> >
> > Fixes sparse warning:
> > arch/x86/kernel/apic/x2apic_cluster.c:13:1: warning: symbol 'per_cpu__x86_cpu_to_logical_apicid' was not declared. Should it be static?
> >
> > Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
> > ---
> > arch/x86/kernel/apic/x2apic_cluster.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
>
> Applied, thanks.
>
> There is a not so small nit:
>
> > Impact: reduce kernel size a bit, avoid sparse warning
> >
> > Fixes sparse warning:
>
> the thing is, we dont 'fix', nor do we 'avoid' Sparse warnings!
>
> We _read_ them, _understand_ them, and then we act upon them, fixing
> the problem they expose.
>
> Or, if there is no problem exposed, we annotate the code to fix the
> Sparse false positive warning.
>
> Your changelog does not tell us anything whether you went through
> that thought process. I had to double-check it and had to create
> this information from scratch.
>
> Please take this as a last warning: you send lots of patches that
> address various things mechanically, often without thinking through
> the effects. They are expensive to maintain, because they cause
> churn and because people often have to do more work accepting them
> than you did creating them!
>
> You sent a hundred patches in two weeks and they are not applied yet
> - and this is why: it is expensive to filter through them and if you
> dont do it we can only do it by simply not taking them all that
> easily. Taking them simply does not scale.
>
> And if you write a hundred patches in two weeks you _really_ have to
> ask yourself whether your quality controls are strong enough before
> emitting them. There are highly productive members of the Linux
> community who only send a dozen patches per _year_.
>
OK, I will be more careful and spend more time on each patch by this way
count will be reduce and quality will also improve.
Please check [git-pull -tip] x86: declaration patches
Sam and Thomas reviewed them and I also fixed the pointed issues.
My problem is I am work-addict I can not sit ideal ;-)
Thanks for your advice,
--
JSR
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-04-12 11:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-04-11 7:25 [PATCH -tip] x86: apic/x2apic_cluster.c x86_cpu_to_logical_apicid should be static Jaswinder Singh Rajput
2009-04-12 10:39 ` [tip:x86/cleanups] " Jaswinder Singh Rajput
2009-04-12 10:51 ` [PATCH -tip] " Ingo Molnar
2009-04-12 11:47 ` Jaswinder Singh Rajput
2009-04-12 11:21 ` [tip:x86/cleanups] " Jaswinder Singh Rajput
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox