All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Jens Axboe <jens.axboe@oracle.com>
Cc: Tejun Heo <tj@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>, Yinghai Lu <yinghai@kernel.org>
Subject: Re: [bug] latest -git boot hang
Date: Sat, 11 Oct 2008 16:58:23 +0200	[thread overview]
Message-ID: <20081011145823.GA14062@elte.hu> (raw)
In-Reply-To: <20081011143947.GT19428@kernel.dk>


* Jens Axboe <jens.axboe@oracle.com> wrote:

> On Sat, Oct 11 2008, Ingo Molnar wrote:
> > 
> > * Jens Axboe <jens.axboe@oracle.com> wrote:
> > 
> > > On Sat, Oct 11 2008, Ingo Molnar wrote:
> > > > 
> > > > * Tejun Heo <tj@kernel.org> wrote:
> > > > 
> > > > > > It does sound like perhaps the option should be hidden more, if it's 
> > > > > > really only reasonably enabled for some very specialized distro 
> > > > > > debuggers, not normal kernel people.
> > > > > 
> > > > > Yeap, if fedora didn't work, I think it should be hidden.  Do we 
> > > > > already have place to hide things like this?
> > > > 
> > > > in my local testing i'm using simple annotations like the one attached 
> > > > further below. Any objections against sending my BROKEN_BOOT_ALLOWED kit 
> > > > upstream, and merge my annotations for various kernel features that 
> > > > break a generic distro bootup?
> > > > 
> > > > Right now i have about 40 such annotations for -tip testing:
> > > > 
> > > >   fs/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   fs/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   security/selinux/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   security/smack/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   security/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/net/appletalk/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/net/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/media/video/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/scsi/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/watchdog/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/watchdog/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/ide/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/i2c/busses/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/block/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/video/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/video/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/video/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/video/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/video/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/video/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/video/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/video/console/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/video/console/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/mtd/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   drivers/isdn/icn/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   lib/Kconfig.kgdb:	depends on BROKEN_BOOT_ALLOWED
> > > >   lib/Kconfig.debug:	depends on BROKEN_BOOT_ALLOWED
> > > >   lib/Kconfig.debug:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig.debug:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig:	# depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig:	depends on BROKEN_BOOT_ALLOWED
> > > >   arch/x86/Kconfig.cpu:	depends on BROKEN_BOOT_ALLOWED
> > > > 
> > > > and note the stark contrast to CONFIG_BROKEN - sometimes a given 
> > > > functionality is really not meant to be enabled on a generic system.
> > > > 
> > > > 	Ingo
> > > > 
> > > > ---------------->
> > > > Subject: qa: no ext devt
> > > > From: Ingo Molnar <mingo@elte.hu>
> > > > Date: Fri Oct 10 22:54:57 CEST 2008
> > > > 
> > > > Signed-off-by: Ingo Molnar <mingo@elte.hu>
> > > > ---
> > > >  lib/Kconfig.debug |    2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > Index: linux/lib/Kconfig.debug
> > > > ===================================================================
> > > > --- linux.orig/lib/Kconfig.debug
> > > > +++ linux/lib/Kconfig.debug
> > > > @@ -670,6 +670,8 @@ config DEBUG_BLOCK_EXT_DEVT
> > > >          bool "Force extended block device numbers and spread them"
> > > >  	depends on DEBUG_KERNEL
> > > >  	depends on BLOCK
> > > > +	depends on BROKEN_BOOT_ALLOWED
> > > > +	select BROKEN_BOOT
> > > >  	default n
> > > >  	help
> > > >  	  Conventionally, block device numbers are allocated from
> > > 
> > > What is BROKEN_BOOT_ALLOWED? Honestly, I'd prefer to just put an extra 
> > > 2-3 line paragraph in the help entry, saying that it's quite possible 
> > > that current distros wont boot with this testing code enabled. Since 
> > > it default to 'n', people should read the entry before turning it on 
> > > anyway.
> > 
> > well, the extra BROKEN_BOOT_ALLOWED helps my automated test-setup to 
> > decide whether a .config that it's testing (either sent by a reporter or 
> > generated randomly) can be booted.
> > 
> > If CONFIG_BROKEN_BOOT_ALLOWED=y, then i allow config options that can 
> > break the bootup. In that case, and _if_ such a possibly-boot-breaking 
> > config option is enabled, CONFIG_BROKEN_BOOT is set - which my scripts 
> > detect.
> > 
> > This gives the test harness the highest flexibility and annotates those 
> > kernel features / drivers which can result in a (possibly) broken 
> > bootup. The scripts cannot read help entries.
> 
> OK, makes sense to me then, thanks. I was afraid it was some user
> exposed parameter, in which case it sounded... interesting :-)
> 
> For users, we just need to expand the help entry a bit.

We could perhaps rename it to:

  CONFIG_ALLOW_NON_GENERIC_FEATURES=y

?

It's usually things like ISA drivers or very specific hardware support 
that falls into this category - none of our major features or drivers, 
so you should not be worried about any limitation effects of such a 
feature. The help text does not help me much in that case, it was not me 
who enabled that option, i just want to use a .config from some other 
person and want to reproduce a bugreport. I do that almost on a daily 
basis.

And this CONFIG_ALLOW_NON_GENERIC_FEATURES=y option could even be 
exposed to users. I have three first-hand usecases for it:

First usecase: when i get a .config from a tester and want to test-boot 
it on a box, i dont want to spend hours of .config bisection just to 
find out that it has a driver enabled that is known to break generic 
boxes. Yes, this has happened to me in the past.

The second usecase where i utilize it is random kernel testing: there 
randconfig is what enables drivers, not me, so the help text does not 
help much.

Third usecase: where i just accidentally enable something i should not 
have enabled. It's nice to have tools around that can protect me from 
such mistakes. This too has happened to me.

So i find it very convenient that i can just disable 
CONFIG_ALLOW_NON_GENERIC_FEATURES - which automatically disables all 
possibly-broken functionality.

	Ingo

  reply	other threads:[~2008-10-11 14:58 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-10 20:30 [bug] latest -git boot hang Ingo Molnar
2008-10-10 20:40 ` Ingo Molnar
2008-10-10 20:56   ` Ingo Molnar
2008-10-10 21:10     ` Linus Torvalds
2008-10-11  0:50       ` Tejun Heo
2008-10-11  1:42         ` H. Peter Anvin
2008-10-13  9:13           ` Tejun Heo
2008-10-13 11:07             ` [PATCH] block: add partition attribute for partition number Tejun Heo
2008-10-13 11:27               ` Jens Axboe
2008-10-11  7:11         ` [bug] latest -git boot hang Jens Axboe
2008-10-11  7:19         ` Ingo Molnar
2008-10-11 14:08           ` Jens Axboe
2008-10-11 14:33             ` Ingo Molnar
2008-10-11 14:39               ` Jens Axboe
2008-10-11 14:58                 ` Ingo Molnar [this message]
2008-10-11 17:58                   ` Jens Axboe
2008-10-13  6:41                     ` [PATCH] block: add BIG FAT WARNING to CONFIG_DEBUG_BLOCK_EXT_DEVT Tejun Heo
2008-10-13  8:46                       ` Jens Axboe
2008-10-14 15:15                       ` Christoph Hellwig
2008-10-14 17:31                         ` Tejun Heo
2008-10-14 15:07           ` [bug] latest -git boot hang Stephen Smalley
2008-10-14 15:12             ` Ingo Molnar
2008-10-14 15:24               ` Stephen Smalley
2008-10-15  1:48                 ` Casey Schaufler
2008-10-13  8:41         ` Takashi Iwai

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=20081011145823.GA14062@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tj@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=yinghai@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.