public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Tejun Heo <tj@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Lee Schermerhorn <Lee.Schermerhorn@hp.com>,
	Christoph Lameter <cl@linux-foundation.org>
Subject: Re: [RFC] remove implicit slab.h inclusion from percpu.h
Date: Tue, 16 Mar 2010 09:16:23 +0100	[thread overview]
Message-ID: <20100316081623.GF18448@elte.hu> (raw)
In-Reply-To: <b6fcc0a1003160014h79098b50t87a448285350b3e@mail.gmail.com>


* Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Tue, Mar 16, 2010 at 8:17 AM, Ingo Molnar <mingo@elte.hu> wrote:
> > Also, why should we make this opt-in and expose a wide range of configs to
> > build breakages?
> 
> Because we want to #include less, not the same amount.
> 
> > A more gradual approach would be to write a simple script
> > that adds a slab.h include to all .c's that include percpu.h, directly or
> > indirectly.
> 
> As a defensive measure, one can explicitly add slab.h to every file which 
> uses kmalloc/kfree.
> 
> But, who cares, since one still has to compile test regardless.

Firstly, generating #include slab.h lines in .c files based on actual API 
usage is _good_, pretty much unconditionally so. See my previous mail for the 
list of advantages. Even if it increases the number of #include lines 
temporarily.

Secondly, the point in scripting is to intentionally over-shoot the target, as 
compile testing (especially if it's only modconfig), will only cover so much 
and _if we can_ we should avoid breakage.

That way the over-shooting will cover cases you are not able (or did not 
happen to) build-test. It's much better to have temporarily more include lines 
than have fewer and break the build. We'll need a #include line reduction 
mechanism in any case as APIs frequently get unused and #include lines get 
amassed in .c files.

Thirdly, as for build testing only, see for example a sched.h include file 
cleanup _you_ did in the past, which, despite your build testing, broke quite 
some code:

 commit 86ae13b006e48959981248493efd3ff4b2828b3d
 Author: Ingo Molnar <mingo@elte.hu>
 Date:   Mon Oct 12 16:22:46 2009 +0200

    headers: Fix build after <linux/sched.h> removal
    
    Commit d43c36dc6b357fa1806800f18aa30123c747a6d1 ("headers: remove
    sched.h from interrupt.h") left some build errors in some configurations
    due to drivers having depended on getting header files "accidentally".
    

    Signed-off-by: Ingo Molnar <mingo@elte.hu>
    [ Combined several one-liners from Ingo into one single patch  - Linus ]
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

 drivers/char/genrtc.c        |    1 +
 drivers/char/rtc.c           |    1 +
 drivers/char/sonypi.c        |    1 +
 drivers/net/wan/c101.c       |    1 +
 drivers/net/wan/n2.c         |    1 +
 drivers/net/wan/pci200syn.c  |    1 +
 drivers/pci/hotplug/cpqphp.h |    1 +
 7 files changed, 7 insertions(+), 0 deletions(-)

I dont mind the occasional breakage, but avoidable bugs spread out in the tree 
can be a real PITA to testers.

I remember it, that imperfect conversion kept me busy chasing trivial build 
bugs for most of the day, because they would pop up with different config 
dependencies so they had to be addressed separately. (so i created 7 separate 
fixes as the day progressed - and that in a busy period of the cycle so i 
certainly didnt have a day to waste voluntarily)

Note that that d43c36dc patch of yours wasnt in linux-next IIRC, so you didnt 
even get basic build coverage. I have no problem with fixing more difficult 
bugs or with fixing the occasional silly oversight (that's the _point_ of 
developing the kernel after all), but i have a problem with reasonably 
avoidable bugs.

Thanks,

	Ingo

  reply	other threads:[~2010-03-16  8:16 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-03-11 14:56 [RFC] remove implicit slab.h inclusion from percpu.h Tejun Heo
2010-03-11 17:48 ` Alexey Dobriyan
2010-03-11 22:33   ` Tejun Heo
2010-03-16  4:27 ` Tejun Heo
2010-03-16  6:17   ` Ingo Molnar
2010-03-16  6:54     ` Tejun Heo
2010-03-16  7:44       ` Tejun Heo
2010-03-16  7:57         ` Ingo Molnar
2010-03-16  8:32           ` Alexey Dobriyan
2010-03-16  9:11             ` Pekka Enberg
2010-03-16  7:49       ` Ingo Molnar
2010-03-16  6:58     ` Pekka Enberg
2010-03-16  7:15       ` Alexey Dobriyan
2010-03-16  7:56         ` Pekka Enberg
2010-03-16  8:23           ` Alexey Dobriyan
2010-03-16  9:06             ` Pekka Enberg
2010-03-16  8:25           ` Ingo Molnar
2010-03-16  7:14     ` Alexey Dobriyan
2010-03-16  8:16       ` Ingo Molnar [this message]
2010-03-16 16:16 ` Christoph Lameter
2010-03-16 22:57   ` Tejun Heo
2010-03-17 16:34     ` Christoph Lameter
2010-03-17 17:14       ` Lee Schermerhorn
2010-03-17 19:54         ` Christoph Lameter
2010-03-17 23:00           ` Tejun Heo

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=20100316081623.GF18448@elte.hu \
    --to=mingo@elte.hu \
    --cc=Lee.Schermerhorn@hp.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --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