mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Andi Kleen <ak@suse.de>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>, Ingo Molnar <mingo@elte.hu>
Subject: Re: [PATCH x86] [12/16] Optimize lock prefix switching to run less frequently
Date: Fri, 4 Jan 2008 23:11:57 +0100	[thread overview]
Message-ID: <200801042311.57837.ak@suse.de> (raw)
In-Reply-To: <alpine.LFD.0.99999.0801041546230.4042@localhost.localdomain>

On Friday 04 January 2008 21:34:15 Thomas Gleixner wrote:
> On Fri, 4 Jan 2008, Andi Kleen wrote:
> > > The kernel process request that _all_ contributors run their patches 
> > > through checkpath.pl and fix the problems.
> > 
> > That's new. When and by whom was that rule been introduced? And with what
> > rationale?
> 
> Documentation/SubmitChecklist

That says

" You should be able to justify all violations that remain in your patch."

I think I did that.

> > Anyways if you care so much about space<->tabs why don't you just write
> > a filter that converts spaces to tabs for incoming patches? Like it is
> > traditionally done for trailing white spaces? Would be a trivial perl script.
> 
> I need to fix up your sloppiness ?

That's the wrong way to see it.  

See it this way: Is forcing humans to convert spaces to tabs an useful activity?
Is rejecting patches for that and requiring repost a useful activity that 
improves Linux in any way? Will it help Linux to let people spend time
to convert spaces to tabs instead of writing patches or testing?

I don't think it is.  Arguably having tabs is nicer than spaces (I wont'
disagree with that).

But since it's something a simple script can do it's 
best to just handle it automatically. 
Then everybody can forget about that and it won't bother anymore again.

ftp://ftp.firstfloor.org/pub/ak/perl/converttabs

Anyways I could run converttabs over my pile and repost if you want,
but as pointed out elsewhere I think it would be better to just integrate
it into the merge flow -- then people could just forget about this forever.

Anyways my future patches will be always run through that.

> The only thing which is beyond ridiculous is your absolute refusal to
> play by the rules.

I question newly introduced rules that don't make sense to me. e.g the absolute
requirement of 100% checkpatch.pl compliance is certainly new.
 
> I just pointed out broken logic in your patch (vs. max_cpus) and your
> answer is just sloppy hand waving ignoring my completely valid
> point. After I pointed it out to you, you do not even have the modesty
> of acknowledging your error. 

I just fixed the patch instead. But you're right I was wrong on that.

> No, a second later you claim that we care 
> only about coding style and not the patch content itself.
> 
> Your findings of broken patches in the x86.git tree just prove that we
> are all human beings, 

Sure that's expected and I missed issues too when I was reviewing x86 patches
(the sentence came out perhaps a more harsh than originally intend) 

But my impression from reading the reviews was that a lot of the rejections
were based only on checkpatch.pl and not on logic checks and there were certainly
a few patches that clearly weren't logic reviewed.  If you do logic checks then that's 
fine of course -- feel free to prove me wrong on that front in the future.

To be honest I was also a little frustrated for patches that I consider
important cleanups (like the early-reserve code) I just get some checkpatch.pl 
complaints.

> but at least the people responsible for the 
> x86.git tree are able to admit their mistakes and fix them without
> arguing.

I fixed all objections that made sense, haven't reposted everything changed yet though.

> On the other side I'm waiting since month for any response 
> from you on a review for a pile of obviously broken patches which you
> wanted to push into 2.6.24.

What patches do you mean? I'm not of anything pending for .24.
 
> Your hyprocritical crusade 

I would prefer calling it "trying to improve inefficient newly introduced procedures
by constructive criticism" :-)

> is annoying the hell out of me, but feel 
> free to ridicule yourself further.

I'm sorry that you don't see my points. I guess I'll need to do a better job
at explaining them, but probably not tonight.

-Andi
 

  reply	other threads:[~2008-01-04 22:13 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 [this message]
2008-01-05  1:19               ` Ingo Molnar
2008-01-05 14:37                 ` Andi Kleen
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=200801042311.57837.ak@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

Powered by JetHome