mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: Ingo Molnar <mingo@elte.hu>
Cc: Andi Kleen <ak@suse.de>, Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH x86] [12/16] Optimize lock prefix switching to run less frequently
Date: Sat, 5 Jan 2008 15:37:28 +0100	[thread overview]
Message-ID: <20080105143728.GB2998@bingen.suse.de> (raw)
In-Reply-To: <20080105011949.GA5107@elte.hu>

> Sloppy and inconsistent code also leaves a lasting negative impression 
> on newbie developers. I've heard quite a few opinions about Linux's code 
> base being a lot less consistent (and hence harder to understand) than 

Do you think they did refer to code style (as in indentation etc.) here? 
When I was talking to such people my impression was that their complaints
usually came from other issues.

e.g. I know from talking to a few people that they consider the BSD
kernels cleaner just because they have a very consistent printk
style for driver probing so it looks cleaner at booting. Sounds silly
(although I can sympathize a little bit) but true.

Besides the kernel indentation is not that inconsistent anyways.

> Frankly, if you dont understand the necessity of clean code, and the 
> negative effect that a large number of small uncleanlinesses can have on 
> a large system then you wont be getting the big picture either.
> 
> checkpatch.pl is not perfect (and it cannot be), and we dont use it as a 
> rigid criterium (and dont want to), but it's quite conservative (its 
> false positive rate for code that i maintain is below 1% - which is 

Rate is certainly higher here.

> sloppy patches. It gives us a way to improve the kernel's code quality 
> gradually. We change about 10% of the kernel codebase in every kernel 
> release, that is about 30% of the kernel every year. With checkpatch.pl 
> we'll have a very clean kernel in just a few years.

That's the problem I have with your position. You equate quality
with coding style or checkpatch.pl output. I think it's only a small
part of it.

If we follow your description above to the logical conclusion then we could 
get the perfect kernel(tm) very quickly just by running everything 
through Lindent or recruit a few hundred people who run all files 
through checkpatch.pl and fix all warnings and then we end up with the 
perfect kernel with "improved code quality" 

Great idea! And is it ok for me to refer to that plan as "the Ingo Molnar
plan" (TIMP) in the future? I'm sure a short term for it will come handy 
in future discussions ;-)

-Andi


  reply	other threads:[~2008-01-05 14:37 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-03 15:42 [PATCH x86] [0/16] Various i386/x86-64 changes Andi Kleen
2008-01-03 15:42 ` [PATCH x86] [1/16] Make clocksource watchdog cycle through online CPUs Andi Kleen
2008-01-04  8:51   ` Ingo Molnar
2008-01-04 12:59     ` Andi Kleen
2008-01-03 15:42 ` [PATCH x86] [2/16] Add a counter for per cpu clocksource watchdog checks and report them in /proc/interrupts Andi Kleen
2008-01-04  8:53   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [3/16] Turn irq debugging options into module params Andi Kleen
2008-01-04  8:55   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [4/16] Add /proc/irq/*/spurious to dump the spurious irq debugging state Andi Kleen
2008-01-04  8:58   ` Ingo Molnar
2008-01-04 12:22     ` Andi Kleen
2008-01-03 15:42 ` [PATCH x86] [5/16] Replace hard coded reservations in x86-64 early boot code with dynamic table Andi Kleen
2008-01-04  9:00   ` Ingo Molnar
2008-01-04 12:23     ` Andi Kleen
2008-01-04 12:38     ` [PATCH x86] [5/16] Replace hard coded reservations in x86-64 early boot code with dynamic table II Andi Kleen
2008-01-04 14:41       ` Ingo Molnar
2008-01-04 15:03         ` Andi Kleen
2008-01-04 17:51           ` Sam Ravnborg
2008-01-04 18:06             ` Andi Kleen
2008-01-04 18:47               ` Joe Perches
2008-01-04 22:13                 ` Andi Kleen
2008-01-04 22:46                   ` J. Bruce Fields
2008-01-04 20:56               ` Sam Ravnborg
2008-01-04 12:41   ` [PATCH x86] [5/16] Replace hard coded reservations in x86-64 early boot code with dynamic table Cyrill Gorcunov
2008-01-03 15:42 ` [PATCH x86] [6/16] Add a new arch_early_alloc() interface for x86-64 Andi Kleen
2008-01-03 16:22   ` Eric Dumazet
2008-01-03 17:17     ` Andi Kleen
2008-01-03 15:42 ` [PATCH x86] [7/16] Convert lockdep to use arch_early_alloc() if available for its large arrays Andi Kleen
2008-01-04  9:03   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [8/16] Make lockdep_init __init Andi Kleen
2008-01-04  9:06   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [9/16] Remove CPU capabitilites printks on i386 Andi Kleen
2008-01-04  9:11   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [10/16] Document fdimage/isoimage completely in make help Andi Kleen
2008-01-04  9:00   ` Sam Ravnborg
2008-01-04  9:15     ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [11/16] Compile apm and voyager module only when selected in Kconfig Andi Kleen
2008-01-04  4:15   ` Stephen Rothwell
2008-01-03 15:42 ` [PATCH x86] [12/16] Optimize lock prefix switching to run less frequently Andi Kleen
2008-01-04  9:42   ` Thomas Gleixner
2008-01-04 12:17     ` Andi Kleen
2008-01-04 14:19       ` Thomas Gleixner
2008-01-04 14:39         ` Andi Kleen
2008-01-04 15:02           ` Pekka Enberg
2008-01-04 15:04             ` Andi Kleen
2008-01-04 20:34           ` Thomas Gleixner
2008-01-04 22:11             ` Andi Kleen
2008-01-05  1:19               ` Ingo Molnar
2008-01-05 14:37                 ` Andi Kleen [this message]
2008-01-03 15:42 ` [PATCH x86] [13/16] i386: Set CFQ as default in i386 defconfig Andi Kleen
2008-01-04  9:19   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [14/16] Enable RDC321X subarch only on 32bit Andi Kleen
2008-01-04  9:22   ` Ingo Molnar
2008-01-03 15:42 ` [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU Andi Kleen
2008-01-03 17:22   ` Rafael J. Wysocki
2008-01-03 17:42     ` Andi Kleen
2008-01-03 21:41       ` Rafael J. Wysocki
2008-01-04  9:23         ` Ingo Molnar
2008-01-10 17:14           ` Rafael J. Wysocki
2008-01-03 18:14   ` Adrian Bunk
2008-01-03 18:43     ` Andi Kleen
2008-01-10  9:54       ` Adrian Bunk
2008-01-10  9:58         ` Andi Kleen
2008-01-10 10:19           ` Adrian Bunk
2008-01-10 11:15             ` Andi Kleen
2008-01-10 11:26               ` Adrian Bunk
2008-01-10 11:42                 ` Andi Kleen
2008-01-10 12:47                   ` Adrian Bunk
2008-01-10 13:12                     ` Andi Kleen
2008-01-10 15:09                       ` Adrian Bunk
2008-01-14 13:52                         ` Ingo Molnar
2008-01-14 14:09                           ` Sam Ravnborg
2008-01-14 14:58                             ` Ingo Molnar
2008-01-14 15:01                               ` Ingo Molnar
2008-01-14 18:20                                 ` Adrian Bunk
2008-01-14 19:10                                   ` Ingo Molnar
2008-01-14 19:52                                     ` Adrian Bunk
2008-01-14 20:01                                       ` Sam Ravnborg
2008-01-14 20:18                                         ` Adrian Bunk
2008-01-14 20:27                                           ` Sam Ravnborg
2008-01-14 20:34                                             ` Adrian Bunk
2008-01-15 22:14                                           ` Ingo Molnar
2008-01-15 22:51                                             ` Adrian Bunk
2008-01-14 19:59                                 ` Sam Ravnborg
2008-01-15 22:12                                   ` [patch for v2.6.24] fix section mismatch warning in page_alloc.c Ingo Molnar
2008-01-14 15:05                               ` [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU Ingo Molnar
2008-01-14 15:24                                 ` Ingo Molnar
2008-01-14 20:12                                   ` Sam Ravnborg
2008-01-15 15:17                                     ` Ingo Molnar
2008-01-15 16:25                                       ` Sam Ravnborg
2008-01-15 17:11                                         ` Andi Kleen
2008-01-15 17:27                                           ` Adrian Bunk
2008-01-15 18:21                                           ` Sam Ravnborg
2008-01-15 18:29                                             ` Andi Kleen
2008-01-15 18:31                                               ` Sam Ravnborg
2008-01-15 18:47                                             ` Adrian Bunk
2008-01-14 18:17                               ` Adrian Bunk
2008-01-14 15:25                           ` Adrian Bunk
2008-01-10 17:17                   ` Rafael J. Wysocki
2008-01-11 23:06                     ` [PATCH] x86: Change unnecessary dependencies on CONFIG_PM Rafael J. Wysocki
2008-01-03 15:42 ` [PATCH x86] [16/16] Mark memory_setup __init Andi Kleen
2008-01-04  9:25   ` Ingo Molnar
2008-01-04  9:53     ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080105143728.GB2998@bingen.suse.de \
    --to=ak@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®