public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: Christoph Hellwig <hch@infradead.org>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@zip.com.au>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Wessel <jason.wessel@windriver.com>
Subject: Re: [patch] kgdb light, v6
Date: Sun, 10 Feb 2008 22:31:20 +0100	[thread overview]
Message-ID: <20080210213120.GD29507@elte.hu> (raw)
In-Reply-To: <200802102043.58895.bzolnier@gmail.com>


* Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> > no. These are not DocBook comments, if you look carefully at the 
> > format [it's not a leading '/**' comment block]. But obviously 
> > documenting this in the include file is very useful, because that's 
> > where people look first, so i kept it. (the APIs will not deviate 
> > across architectures)
> 
> comments and variable names in include files have a tendency for going 
> out-of-sync in the long term so IMO having a DocBook to point people 
> at would be a better solution (+ it would shrink <linux/kgdb.h> by 122 
> lines)

yes, i very much agree in general, but this is a _SPECIAL CASE_, and i 
already tried to point that out to Christoph but he's not the type of 
guy who listens to others all that easily when it comes to his pet 
peeves ;-)

this is a special case because it's an _architecture facility_.

read: right now we have 25 architectures, and this means that in a year 
we'll have 25 arch/*/kernel/kgdb.c files. What will be more likely to 
get out of sync, 25 full sets of DocBook entries of the same thing, 
spread across 25 architectures - or that lone single 
include/linux/kgdb.h file that is looked at by everyone? And what will 
be easier to update if we extend any of the APIs?

so the DocBook rules are fine, but in this SPECIAL CASE they cause the 
possibly worst solution: total information anarchy!

the correct approach is to put the _arch specific_ details into the 
arch/*/kernel/kgdb.c files, and to keep the generic bits in 
include/linux/kgdb.h. KGDB did exactly that and it's by far the cleanest 
and most maintainable approach.

If DocBook does not pick that up then it's a _DocBook problem_. I dont 
mind adding some dummy weak aliases to kernel/kgdb.c for DocBook to pick 
up, to help solve this DocBook problem - but to blame it on KGDB is way 
off the mark. It used to be the crappiest piece of sh*t everyone would 
laugh about when looking at (right before suffering permanent brain 
damage), but now it's one of the cleanest and most CodingStyle conform 
kernel subsystems :-)

case in point:

                             errors   lines of code   errors/KLOC
  kernel/kgdb.c                   0            1839             0
  fs/xfs/                      2102          106019          19.8

right, XFS has more than 2 thousand bona fide CodingStyle violations! 

But yeah, it has the luxory of upstream integration ... ;-)

[ not that i want to pick on XFS - it's a very clean codebase in my 
  opinion, considering its fundamental complexity. It's just that anyone 
  who wants to find a style error in KGDB now has to search _hard_ ;-) ]

	Ingo

  reply	other threads:[~2008-02-10 21:31 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-10  7:13 [3/6] kgdb: core Ingo Molnar
2008-02-10  7:31 ` Sam Ravnborg
2008-02-10  7:59   ` Ingo Molnar
2008-02-10  7:35 ` Christoph Hellwig
2008-02-10  7:43   ` Ingo Molnar
2008-02-10  7:57     ` Christoph Hellwig
2008-02-10  8:02       ` Ingo Molnar
2008-02-10  8:21         ` Ingo Molnar
2008-02-10  8:26           ` Christoph Hellwig
2008-02-10  9:08             ` Ingo Molnar
2008-02-10  9:17               ` Ingo Molnar
2008-02-10  9:20                 ` Ingo Molnar
2008-02-10  9:34                   ` Ingo Molnar
2008-02-10  9:31                 ` Christoph Hellwig
2008-02-10 17:17                   ` [patch] kgdb light, v6 Ingo Molnar
2008-02-10 19:43                     ` Bartlomiej Zolnierkiewicz
2008-02-10 21:31                       ` Ingo Molnar [this message]
2008-02-10 20:55                     ` Bartlomiej Zolnierkiewicz
2008-02-10 21:09                       ` Ingo Molnar
2008-02-10 21:45                         ` Jan Kiszka
2008-02-10 22:14                           ` Bartlomiej Zolnierkiewicz
2008-02-10 22:32                             ` Jan Kiszka
2008-02-10 22:40                               ` Ingo Molnar
2008-02-11  2:35                                 ` Yinghai Lu
2008-02-10 22:31                           ` Ingo Molnar
2008-02-10 22:24                         ` Bartlomiej Zolnierkiewicz
2008-02-10  8:24         ` [3/6] kgdb: core Christoph Hellwig
2008-02-10  8:57           ` Ingo Molnar
2008-02-10  9:11             ` Christoph Hellwig
2008-02-10  9:27               ` Ingo Molnar
2008-02-10  9:34                 ` Christoph Hellwig
2008-02-10 17:02                   ` Ingo Molnar
2008-02-10 12:46 ` Marcin Slusarz
2008-02-10 13:19   ` Jesper Juhl
2008-02-10 14:00     ` Marcin Slusarz
2008-02-10 13:36   ` Jan Kiszka
2008-02-10 16:43   ` Ingo Molnar
2008-02-10 19:20     ` Bartlomiej Zolnierkiewicz
2008-02-10 16:46   ` 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=20080210213120.GD29507@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@zip.com.au \
    --cc=bzolnier@gmail.com \
    --cc=hch@infradead.org \
    --cc=jason.wessel@windriver.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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