Re: linux-next: Tree for August 27

Re: linux-next: Tree for August 27

[Bartlomiej Zolnierkiewicz]

On Wednesday 27 August 2008, Bartlomiej Zolnierkiewicz wrote:
> On Wed, Aug 27, 2008 at 12:06 PM, Bartlomiej Zolnierkiewicz
> <bzolnier@gmail.com> wrote:
> >
> > Hi,
> >
> > On Wednesday 27 August 2008, Stephen Rothwell wrote:
> >> Hi all,
> >>
> >> Changes since next-20080826:
> >
> > it fails to build here with:
> >
> > arch/x86/kernel/cpu/addon_cpuid_features.c: In function 'detect_extended_topology':
> > arch/x86/kernel/cpu/addon_cpuid_features.c:119: error: 'struct cpuinfo_x86' hasno member named 'cpu_core_id'
> > arch/x86/kernel/cpu/addon_cpuid_features.c:121: error: 'struct cpuinfo_x86' hasno member named 'phys_proc_id'
> > arch/x86/kernel/cpu/addon_cpuid_features.c:130: error: 'struct cpuinfo_x86' hasno member named 'phys_proc_id'
> > arch/x86/kernel/cpu/addon_cpuid_features.c:133: error: 'struct cpuinfo_x86' hasno member named 'cpu_core_id'
> > make[2]: *** [arch/x86/kernel/cpu/addon_cpuid_features.o] Error 1
> > make[1]: *** [arch/x86/kernel/cpu] Error 2
> > make: *** [arch/x86/kernel] Error 2
> 
> which is most likely already fixed in x86 tree by
> 
> http://lkml.org/lkml/2008/8/27/34

Confirmed.

However now I'm getting device-mapper errors on 'cryptsetup create':

dmesg:
...
device-mapper: core: bdget failed in dm_suspend
device-mapper: ioctl: device doesn't appear to be in the dev hash table.
...

cryptsetup:
...
Command failed: device-mapper: resume ioctl failed: Invalid argument
...

next-20080826 is fine

BTW it also seems that certain people need some encouragement
to fix the nasty warning first reported by Randy two weeks ago
(http://lkml.org/lkml/2008/8/14/207) and later addressed by
Rusty (http://lkml.org/lkml/2008/8/16/40) (the solution is said
to be fragile but there doesn't seem to be any better one on
the horizon so...)

Re: linux-next: Tree for August 27

[Bartlomiej Zolnierkiewicz]

[ Thanks to Ingo for handling x86-vs-UP build fix ]

On Wednesday 27 August 2008, Bartlomiej Zolnierkiewicz wrote:

[...]

> However now I'm getting device-mapper errors on 'cryptsetup create':
> 
> dmesg:
> ...
> device-mapper: core: bdget failed in dm_suspend
> device-mapper: ioctl: device doesn't appear to be in the dev hash table.
> ...
> 
> cryptsetup:
> ...
> Command failed: device-mapper: resume ioctl failed: Invalid argument
> ...
> 
> next-20080826 is fine

next-20080828 still isn't, The Lucky Winner is:

commit 9617fc8f8e599166590fe339de78a50bc5ab5e93
Author: Tejun Heo <tj@kernel.org>
Date:   Mon Aug 25 19:56:08 2008 +0900

    block: move __dev from disk to part0

    Move disk->__dev to part0->__dev.  This simplifies bdget_disk() and
    lookup_devt() and allows common sysfs attributes to be unified.
    part_to_disk() is updated to handle part0 -> disk.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Signed-off-by: Jens Axboe <jens.axboe@oracle.com>

Fix:

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] block: fix part0 handling in bdget_disk() / blk_lookup_devt()

part0 is a "special" partition and doesn't need to have capacity set - this
fixes regression caused by "block: move __dev from disk to part0" commit.

Cc: Tejun Heo <tj@kernel.org>
Cc: Jens Axboe <jens.axboe@oracle.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
Jens, please integrate it into the original commit if feasible.

 block/genhd.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: b/block/genhd.c
===================================================================
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -543,7 +543,7 @@ extern struct block_device *bdget_disk(s
 	struct block_device *bdev = NULL;
 
 	part = disk_get_part(disk, partno);
-	if (part && part->nr_sects)
+	if (part && (part->nr_sects || partno == 0))
 		bdev = bdget(part_devt(part));
 	disk_put_part(part);
 
@@ -1001,7 +1001,7 @@ dev_t blk_lookup_devt(const char *name, 
 			continue;
 
 		part = disk_get_part(disk, partno);
-		if (part && part->nr_sects) {
+		if (part && (part->nr_sects || partno == 0)) {
 			devt = part_devt(part);
 			disk_put_part(part);
 			break;
--

On the somehow related topic - CONFIG_DEBUG_BLOCK_EXT_DEVT which is
introduced by commit 3e5ba59bbab878f60b8b87004ac55a464bddc819 ("block:
implement CONFIG_DEBUG_BLOCK_EXT_DEVT") is way too intrusive to be
enabled by default if CONFIG_DEBUG_KERNEL is set as it easily results
in no longer bootable systems (+ help entry for the new option says
"Say N if you are unsure.").

Tejun/Jens: please consider changing this.

> BTW it also seems that certain people need some encouragement
> to fix the nasty warning first reported by Randy two weeks ago
> (http://lkml.org/lkml/2008/8/14/207) and later addressed by
> Rusty (http://lkml.org/lkml/2008/8/16/40) (the solution is said
> to be fragile but there doesn't seem to be any better one on
> the horizon so...)

Solution is indeed very fragile - it managed to convert mere
WARNING into full BUG.  Amazing! ;)

BUG: unable to handle kernel NULL pointer dereference at 00000030
IP: [<c023540a>] driver_register+0x46/0xd6
*pde = 00000000
Oops: 0000 [#1] PREEMPT
last sysfs file:
Modules linked in:

Pid: 1, comm: swapper Not tainted (2.6.27-rc4-next-20080828-dirty #182)
EIP: 0060:[<c023540a>] EFLAGS: 00000246 CPU: 0
EIP is at driver_register+0x46/0xd6
EAX: 00000000 EBX: 00000000 ECX: c7801f60 EDX: c03c1838
ESI: c040807c EDI: c03c1908 EBP: 00000000 ESP: c7820f5c
 DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068
Process swapper (pid: 1, ti=c7820000 task=c781fcc0 task.ti=c7820000)
Stack: 00000000 00000000 c040807c 00000000 00000000 c03e9f00 c0101122 c03e9ebb
       00000001 00000000 c0117300 c0307528 0000000f c04202b8 c782b330 00000001
       00000246 00000246 c780a5a0 c78022e0 00000000 c0355ff8 c0128b07 00000000
Call Trace:
 [<c03e9f00>] acpi_power_init+0x45/0x65
 [<c0101122>] _stext+0x3a/0x12a
 [<c03e9ebb>] acpi_power_init+0x0/0x65
 [<c0117300>] __enqueue_entity+0xcc/0xe2
 [<c0128b07>] __create_workqueue_key+0x14d/0x162
 [<c03d84ba>] kernel_init+0xec/0x150
 [<c03d83ce>] kernel_init+0x0/0x150
 [<c0103a47>] kernel_thread_helper+0x7/0x10
 =======================
Code: 00 74 06 83 7f 14 00 75 0c 83 78 20 00 74 14 83 7f 18 00 74 0e ff 37 68 02 c0 37 c0 e8 13 93 0c 00 5e 5d 8b 47 04 8b 17 8b 40 38 <8b> 40 30 e8 bc bb f9 ff 85 c0 74 24 8b 40 54 85 c0 74 1d 8b 40
EIP: [<c023540a>] driver_register+0x46/0xd6 SS:ESP 0068:c7820f5c
---[ end trace 4eaa2a86a8e2da22 ]---
Kernel panic - not syncing: Attempted to kill init!

Re: linux-next: Tree for August 27

[Jens Axboe]

On Fri, Aug 29 2008, Bartlomiej Zolnierkiewicz wrote:
> 
> [ Thanks to Ingo for handling x86-vs-UP build fix ]
> 
> On Wednesday 27 August 2008, Bartlomiej Zolnierkiewicz wrote:
> 
> [...]
> 
> > However now I'm getting device-mapper errors on 'cryptsetup create':
> > 
> > dmesg:
> > ...
> > device-mapper: core: bdget failed in dm_suspend
> > device-mapper: ioctl: device doesn't appear to be in the dev hash table.
> > ...
> > 
> > cryptsetup:
> > ...
> > Command failed: device-mapper: resume ioctl failed: Invalid argument
> > ...
> > 
> > next-20080826 is fine
> 
> next-20080828 still isn't, The Lucky Winner is:
> 
> commit 9617fc8f8e599166590fe339de78a50bc5ab5e93
> Author: Tejun Heo <tj@kernel.org>
> Date:   Mon Aug 25 19:56:08 2008 +0900
> 
>     block: move __dev from disk to part0
> 
>     Move disk->__dev to part0->__dev.  This simplifies bdget_disk() and
>     lookup_devt() and allows common sysfs attributes to be unified.
>     part_to_disk() is updated to handle part0 -> disk.
> 
>     Signed-off-by: Tejun Heo <tj@kernel.org>
>     Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
> 
> Fix:
> 
> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [PATCH] block: fix part0 handling in bdget_disk() / blk_lookup_devt()
> 
> part0 is a "special" partition and doesn't need to have capacity set - this
> fixes regression caused by "block: move __dev from disk to part0" commit.
> 
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Jens Axboe <jens.axboe@oracle.com>
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
> Jens, please integrate it into the original commit if feasible.

Thanks Bart, I folded it into the original commit.

> 
>  block/genhd.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Index: b/block/genhd.c
> ===================================================================
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -543,7 +543,7 @@ extern struct block_device *bdget_disk(s
>  	struct block_device *bdev = NULL;
>  
>  	part = disk_get_part(disk, partno);
> -	if (part && part->nr_sects)
> +	if (part && (part->nr_sects || partno == 0))
>  		bdev = bdget(part_devt(part));
>  	disk_put_part(part);
>  
> @@ -1001,7 +1001,7 @@ dev_t blk_lookup_devt(const char *name, 
>  			continue;
>  
>  		part = disk_get_part(disk, partno);
> -		if (part && part->nr_sects) {
> +		if (part && (part->nr_sects || partno == 0)) {
>  			devt = part_devt(part);
>  			disk_put_part(part);
>  			break;
> --
> 
> On the somehow related topic - CONFIG_DEBUG_BLOCK_EXT_DEVT which is
> introduced by commit 3e5ba59bbab878f60b8b87004ac55a464bddc819 ("block:
> implement CONFIG_DEBUG_BLOCK_EXT_DEVT") is way too intrusive to be
> enabled by default if CONFIG_DEBUG_KERNEL is set as it easily results
> in no longer bootable systems (+ help entry for the new option says
> "Say N if you are unsure.").
> 
> Tejun/Jens: please consider changing this.

I have change the default to 'n' for now.

-- 
Jens Axboe

Re: linux-next: Tree for August 27

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]

Hi Bart,

On Fri, 29 Aug 2008 09:05:16 +0200 Jens Axboe <jens.axboe@oracle.com> wrote:
>
> On Fri, Aug 29 2008, Bartlomiej Zolnierkiewicz wrote:
> > 
> > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Subject: [PATCH] block: fix part0 handling in bdget_disk() / blk_lookup_devt()
> > 
> > part0 is a "special" partition and doesn't need to have capacity set - this
> > fixes regression caused by "block: move __dev from disk to part0" commit.
> > 
> > Cc: Tejun Heo <tj@kernel.org>
> > Cc: Jens Axboe <jens.axboe@oracle.com>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> > Jens, please integrate it into the original commit if feasible.
> 
> Thanks Bart, I folded it into the original commit.

And applied to today's linux-next.

> > On the somehow related topic - CONFIG_DEBUG_BLOCK_EXT_DEVT which is
> > introduced by commit 3e5ba59bbab878f60b8b87004ac55a464bddc819 ("block:
> > implement CONFIG_DEBUG_BLOCK_EXT_DEVT") is way too intrusive to be
> > enabled by default if CONFIG_DEBUG_KERNEL is set as it easily results
> > in no longer bootable systems (+ help entry for the new option says
> > "Say N if you are unsure.").
> > 
> > Tejun/Jens: please consider changing this.
> 
> I have change the default to 'n' for now.

I will do this for today as well.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[PATCH] block: don't test for partition size in bdget_disk() and blk_lookup_devt()

[Tejun Heo]

bdget_disk() and blk_lookup_devt() never cared whether the specified
partition (or disk) is zero sized or not.  I got confused while
converting those not to depend on consecutive minor numbers in commit
5a6411b1178baf534aa9138052864dfa89d3eada and later when dev0 was added
it broke callers which expected to get valid return for zero sized
disk devices.

So, they never needed nr_sects checks in the first place.  Kill them.

This problem was spotted and debugged by Bartlmoiej Zolnierkiewicz.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
Thanks for the good spotting.  I got confused between the meaning of
!part and !part->nr_sects and added the wrong check during conversion.
I audited the rest and it seems those two are the only ones.

Thanks.

 block/genhd.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: work/block/genhd.c
===================================================================
--- work.orig/block/genhd.c
+++ work/block/genhd.c
@@ -586,7 +586,7 @@ extern struct block_device *bdget_disk(s
 	struct block_device *bdev = NULL;
 
 	part = disk_get_part(disk, partno);
-	if (part && (part->nr_sects || partno == 0))
+	if (part)
 		bdev = bdget(part_devt(part));
 	disk_put_part(part);
 
@@ -1032,7 +1032,7 @@ dev_t blk_lookup_devt(const char *name,
 			continue;
 
 		part = disk_get_part(disk, partno);
-		if (part && (part->nr_sects || partno == 0)) {
+		if (part) {
 			devt = part_devt(part);
 			disk_put_part(part);
 			break;

Re: linux-next: Tree for August 27

[Tejun Heo]

Jens Axboe wrote:
>> On the somehow related topic - CONFIG_DEBUG_BLOCK_EXT_DEVT which is
>> introduced by commit 3e5ba59bbab878f60b8b87004ac55a464bddc819 ("block:
>> implement CONFIG_DEBUG_BLOCK_EXT_DEVT") is way too intrusive to be
>> enabled by default if CONFIG_DEBUG_KERNEL is set as it easily results
>> in no longer bootable systems (+ help entry for the new option says
>> "Say N if you are unsure.").
>>
>> Tejun/Jens: please consider changing this.
> 
> I have change the default to 'n' for now.

Hmmm... it broke booting?  Bart, can you please elaborate a bit what got
broken?  Did it fail to locate the root partition or were you using
static device nodes?

Thanks.

-- 
tejun

Re: linux-next: Tree for August 27

[Jens Axboe]

On Fri, Aug 29 2008, Tejun Heo wrote:
> Jens Axboe wrote:
> >> On the somehow related topic - CONFIG_DEBUG_BLOCK_EXT_DEVT which is
> >> introduced by commit 3e5ba59bbab878f60b8b87004ac55a464bddc819 ("block:
> >> implement CONFIG_DEBUG_BLOCK_EXT_DEVT") is way too intrusive to be
> >> enabled by default if CONFIG_DEBUG_KERNEL is set as it easily results
> >> in no longer bootable systems (+ help entry for the new option says
> >> "Say N if you are unsure.").
> >>
> >> Tejun/Jens: please consider changing this.
> > 
> > I have change the default to 'n' for now.
> 
> Hmmm... it broke booting?  Bart, can you please elaborate a bit what got
> broken?  Did it fail to locate the root partition or were you using
> static device nodes?

It works fine for me, fwiw.

-- 
Jens Axboe

Re: linux-next: Tree for August 27

[Bartlomiej Zolnierkiewicz]

Hi,

On Friday 29 August 2008, Jens Axboe wrote:
> On Fri, Aug 29 2008, Tejun Heo wrote:
> > Jens Axboe wrote:
> > >> On the somehow related topic - CONFIG_DEBUG_BLOCK_EXT_DEVT which is
> > >> introduced by commit 3e5ba59bbab878f60b8b87004ac55a464bddc819 ("block:
> > >> implement CONFIG_DEBUG_BLOCK_EXT_DEVT") is way too intrusive to be
> > >> enabled by default if CONFIG_DEBUG_KERNEL is set as it easily results
> > >> in no longer bootable systems (+ help entry for the new option says
> > >> "Say N if you are unsure.").
> > >>
> > >> Tejun/Jens: please consider changing this.
> > > 
> > > I have change the default to 'n' for now.
> > 
> > Hmmm... it broke booting?  Bart, can you please elaborate a bit what got
> > broken?  Did it fail to locate the root partition or were you using
> > static device nodes?
> 
> It works fine for me, fwiw.

It fails to locate the root partition if you are using the default root
device on x86 (the one hard-wired into bzImage, it uses hard-coded 8-bit
major/minor numbers).

Now I see that the issue is easily fixed by passing the root device name
with "root=" (since it will figure out the actual major/minor numbers
later) and that the default root device is said to be deprecated according
to Documentation/x86/boot.txt (OTOH there is no warning at all that it is
deprecated and that you should be using "root=" so I'm not convinced that
I'm the only one who would hit the unfortunate combination).

Actually it seems that CONFIG_DEBUG_BLOCK_EXT_DEVT introduction (which I
think is a nice step forward) is a good oportunity to put the deprecation
of the default root device further, i.e. it may be worth to hook the
default root device check and/or CONFIG_DEBUG_BLOCK_EXT_DEVT one into
mount_block_root().

Thanks,
Bart

Re: linux-next: Tree for August 27

[Tejun Heo]

Hello,

Bartlomiej Zolnierkiewicz wrote:
>>> Hmmm... it broke booting?  Bart, can you please elaborate a bit what got
>>> broken?  Did it fail to locate the root partition or were you using
>>> static device nodes?
>> It works fine for me, fwiw.
> 
> It fails to locate the root partition if you are using the default root
> device on x86 (the one hard-wired into bzImage, it uses hard-coded 8-bit
> major/minor numbers).

Right.  That would break it too.

> Now I see that the issue is easily fixed by passing the root device name
> with "root=" (since it will figure out the actual major/minor numbers
> later) and that the default root device is said to be deprecated according
> to Documentation/x86/boot.txt (OTOH there is no warning at all that it is
> deprecated and that you should be using "root=" so I'm not convinced that
> I'm the only one who would hit the unfortunate combination).

Yeap, in retrospect, enabling BLOCK_EXT_DEVT by default seems too
disruptive.  There also are people running off static /dev.  I hope
distros enable it during their alphas and betas tho so that they can
spot problems in configuration utilities and stuff early.

> Actually it seems that CONFIG_DEBUG_BLOCK_EXT_DEVT introduction (which I
> think is a nice step forward) is a good oportunity to put the deprecation
> of the default root device further, i.e. it may be worth to hook the
> default root device check and/or CONFIG_DEBUG_BLOCK_EXT_DEVT one into
> mount_block_root().

Maybe we can add a warning there if BLOCK_EXT_DEVT is enabled or do
you have something else on mind?

Thanks.

-- 
tejun

Re: linux-next: Tree for August 27

[Bartlomiej Zolnierkiewicz]

On Saturday 30 August 2008, Tejun Heo wrote:

[...]

> > Actually it seems that CONFIG_DEBUG_BLOCK_EXT_DEVT introduction (which I
> > think is a nice step forward) is a good oportunity to put the deprecation
> > of the default root device further, i.e. it may be worth to hook the
> > default root device check and/or CONFIG_DEBUG_BLOCK_EXT_DEVT one into
> > mount_block_root().
> 
> Maybe we can add a warning there if BLOCK_EXT_DEVT is enabled or do
> you have something else on mind?

Yes, this is would be nice, having warning on using the default root
device (instead of "root=") would be even nicer.

Thanks,
Bart

[PATCH #blk-for-2.8] init: DEBUG_BLOCK_EXT_DEVT requires explicit root= param

[Tejun Heo]

DEBUG_BLOCK_EXT_DEVT shuffles SCSI and IDE device numbers and root
device number set using rdev become meaningless.  Root devices should
be explicitly specified using textual names.  Warn about it if root
can't be found and DEBUG_BLOCK_EXT_DEVT is enabled.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
There are other ways to specify device numbers directly (ie. root=8:0)
and DEBUG_BLOCK_EXT_DEVT being a debug option I don't think it
warrants a complex logic, so I just added simple warning message.

 init/do_mounts.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: work/init/do_mounts.c
===================================================================
--- work.orig/init/do_mounts.c
+++ work/init/do_mounts.c
@@ -263,6 +263,10 @@ retry:
 		printk("Please append a correct \"root=\" boot option; here are the available partitions:\n");
 
 		printk_all_partitions();
+#ifdef CONFIG_DEBUG_BLOCK_EXT_DEVT
+		printk("DEBUG_BLOCK_EXT_DEVT is enabled, you need to specify "
+		       "explicit textual name for \"root=\" boot option.\n");
+#endif
 		panic("VFS: Unable to mount root fs on %s", b);
 	}
 

Re: [PATCH #blk-for-2.8] init: DEBUG_BLOCK_EXT_DEVT requires explicit root= param

[Jens Axboe]

On Mon, Sep 01 2008, Tejun Heo wrote:
> DEBUG_BLOCK_EXT_DEVT shuffles SCSI and IDE device numbers and root
> device number set using rdev become meaningless.  Root devices should
> be explicitly specified using textual names.  Warn about it if root
> can't be found and DEBUG_BLOCK_EXT_DEVT is enabled.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
> There are other ways to specify device numbers directly (ie. root=8:0)
> and DEBUG_BLOCK_EXT_DEVT being a debug option I don't think it
> warrants a complex logic, so I just added simple warning message.

How about just putting that in the help entry as well? Given that it's
now defaulting to 'n', people that enable it should read the help text
anyway.

-- 
Jens Axboe

Re: [PATCH #blk-for-2.8] init: DEBUG_BLOCK_EXT_DEVT requires explicit root= param

[Tejun Heo]

Jens Axboe wrote:
>> There are other ways to specify device numbers directly (ie. root=8:0)
>> and DEBUG_BLOCK_EXT_DEVT being a debug option I don't think it
>> warrants a complex logic, so I just added simple warning message.
> 
> How about just putting that in the help entry as well? Given that it's
> now defaulting to 'n', people that enable it should read the help text
> anyway.

I'll add that to help message but I still think it'll be beneficial to
have warning message printed to avoid getting people off guard. e.g.
alpha/beta distro built with the debug option turned on to detect bugs.

Thanks.

-- 
tejun

Re: [PATCH #blk-for-2.8] init: DEBUG_BLOCK_EXT_DEVT requires explicit root= param

[Jens Axboe]

On Mon, Sep 01 2008, Tejun Heo wrote:
> Jens Axboe wrote:
> >> There are other ways to specify device numbers directly (ie. root=8:0)
> >> and DEBUG_BLOCK_EXT_DEVT being a debug option I don't think it
> >> warrants a complex logic, so I just added simple warning message.
> > 
> > How about just putting that in the help entry as well? Given that it's
> > now defaulting to 'n', people that enable it should read the help text
> > anyway.
> 
> I'll add that to help message but I still think it'll be beneficial to
> have warning message printed to avoid getting people off guard. e.g.
> alpha/beta distro built with the debug option turned on to detect bugs.

Honestly, those distros should not enable that option. But it's not a
big deal, if you send a patch with the Kconfig wording updated as well,
I'll add it.

-- 
Jens Axboe

Re: [PATCH #blk-for-2.8] init: DEBUG_BLOCK_EXT_DEVT requires explicit root= param

[Tejun Heo]

DEBUG_BLOCK_EXT_DEVT shuffles SCSI and IDE device numbers and root
device number set using rdev become meaningless.  Root devices should
be explicitly specified using textual names.  Warn about it if root
can't be found and DEBUG_BLOCK_EXT_DEVT is enabled.  Also, add warning
to the help text.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
I think distros should enable it for early alpha/betas to catch odd
problems.  Well, anyways, here's the updated version.  Thanks.

 init/do_mounts.c  |    4 ++++
 lib/Kconfig.debug |    6 ++++++
 2 files changed, 10 insertions(+)

Index: work/init/do_mounts.c
===================================================================
--- work.orig/init/do_mounts.c
+++ work/init/do_mounts.c
@@ -263,6 +263,10 @@ retry:
 		printk("Please append a correct \"root=\" boot option; here are the available partitions:\n");
 
 		printk_all_partitions();
+#ifdef CONFIG_DEBUG_BLOCK_EXT_DEVT
+		printk("DEBUG_BLOCK_EXT_DEVT is enabled, you need to specify "
+		       "explicit textual name for \"root=\" boot option.\n");
+#endif
 		panic("VFS: Unable to mount root fs on %s", b);
 	}
 
Index: work/lib/Kconfig.debug
===================================================================
--- work.orig/lib/Kconfig.debug
+++ work/lib/Kconfig.debug
@@ -638,6 +638,12 @@ config DEBUG_BLOCK_EXT_DEVT
 	  userland code paths which assume predetermined contiguous
 	  device number allocation.
 
+	  Note that turning on this debug option shuffles all the
+	  device numbers for all IDE and SCSI devices including libata
+	  ones, so root partition specified using device number
+	  directly (via rdev or root=MAJ:MIN) won't work anymore.
+	  Textual device names (root=/dev/sdXn) will continue to work.
+
 	  Say N if you are unsure.
 
 config LKDTM

Re: [PATCH #blk-for-2.8] init: DEBUG_BLOCK_EXT_DEVT requires explicit root= param

[Bartlomiej Zolnierkiewicz]

On Monday 01 September 2008, Tejun Heo wrote:
> DEBUG_BLOCK_EXT_DEVT shuffles SCSI and IDE device numbers and root
> device number set using rdev become meaningless.  Root devices should
> be explicitly specified using textual names.  Warn about it if root
> can't be found and DEBUG_BLOCK_EXT_DEVT is enabled.  Also, add warning
> to the help text.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Thanks Tejun.  With this patch I'm also fine with having
DEBUG_BLOCK_EXT_DEVT enabled by default.