linux-kernel - Re: [PATCH -tip] x86: apic/x2apic_cluster.c x86_cpu_to_logical

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 12 Apr 2009 12:51:32 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Jaswinder Singh Rajput <jaswinder@...nel.org>
Cc:	x86 maintainers <x86@...nel.org>,
	Suresh Siddha <suresh.b.siddha@...el.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Sam Ravnborg <sam@...nborg.org>
Subject: Re: [PATCH -tip] x86: apic/x2apic_cluster.c
	x86_cpu_to_logical_apicid should be static

* Jaswinder Singh Rajput <jaswinder@...nel.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@...il.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
--
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/