MTD-related kobject badness during linux-next boot

MTD-related kobject badness during linux-next boot

[Daniel Drake]

Hi,

Booting linux-next 20110707 on OLPC XO-1 I get:

[    0.965646] CAFE NAND 0000:00:0c.0: enabling device (0000 -> 0002)
[    0.984187] NAND device: Manufacturer ID: 0xad, Chip ID: 0xdc
(Hynix NAND 512MiB 3,3V 8-bit)
[    1.001115] 2 NAND chips detected
[    1.144358] Searching for RedBoot partition table in cafe_nand at offset 0x0
[    1.162364] No RedBoot partition table detected in cafe_nand
[    1.172947] kobject (cd8d40dc): tried to init an initialized
object, something is seriously wrong.
[    1.184111] Pid: 1, comm: swapper Not tainted 3.0.0-rc6-next-20110707+ #100
[    1.198384] Call Trace:
[    1.207741]  [<c05301a0>] ? kobject_init+0x24/0x6b
[    1.212420]  [<c0578c49>] ? device_initialize+0x18/0x56
[    1.227060]  [<c057960a>] ? device_register+0x8/0x10
[    1.241052]  [<c059104a>] ? add_mtd_device+0x17a/0x1f1
[    1.244949]  [<c0591128>] ? mtd_device_parse_register+0x67/0x7c
[    1.259536]  [<c06c6522>] ? cafe_nand_probe+0x5c9/0x686
[    1.263228]  [<c0546697>] ? pci_device_probe+0x52/0xb0
[    1.276828]  [<c057ad8b>] ? driver_probe_device+0x8a/0x109
[    1.290960]  [<c057ae4a>] ? __driver_attach+0x40/0x5b
[    1.294236]  [<c057a370>] ? bus_for_each_dev+0x37/0x60
[    1.307430] Switching to clocksource tsc
[    1.316560]  [<c057ab33>] ? driver_attach+0x14/0x17
[    1.329149]  [<c057ae0a>] ? driver_probe_device+0x109/0x109
[    1.362236] Switched to NOHz mode on CPU #0
[    1.390945]  [<c057a867>] ? bus_add_driver+0x88/0x1b1
[    1.422422]  [<c0530945>] ? kset_find_obj_hinted+0x6f/0x94
[    1.453723]  [<c07ef640>] ? start_kernel+0x293/0x293
[    1.483790]  [<c0803af6>] ? nand_base_init+0x12/0x12
[    1.513128]  [<c057b1ca>] ? driver_register+0x73/0xc6
[    1.541936]  [<c0803af6>] ? nand_base_init+0x12/0x12
[    1.570298]  [<c07ef640>] ? start_kernel+0x293/0x293
[    1.598567]  [<c0803af6>] ? nand_base_init+0x12/0x12
[    1.626318]  [<c0546ea3>] ? __pci_register_driver+0x2c/0x81
[    1.654298]  [<c07ef640>] ? start_kernel+0x293/0x293
[    1.680913]  [<c0803af6>] ? nand_base_init+0x12/0x12
[    1.706828]  [<c0401065>] ? do_one_initcall+0x65/0x104
[    1.733304]  [<c07ef640>] ? start_kernel+0x293/0x293
[    1.758893]  [<c07ef6a5>] ? kernel_init+0x65/0xda
[    1.783437]  [<c06cd9d6>] ? kernel_thread_helper+0x6/0xd

I think this is a recent regression but I haven't been testing enough
recent kernels to know exactly when it appeared.

This seems to cause shutdown to fail as well, as device_shutdown()
gets in an infinite loop trying to shut down mtd1.

Any ideas?

Thanks,
Daniel

Re: MTD-related kobject badness during linux-next boot

[Brian Norris]

Hi,

I CC'd Dmitry and Jamie who did recent work on this area of MTD,
regarding a subsystem-wide simplification of the routines for
partitioning and registering devices. I don't personally develop with
the cafe_nand driver, but I can try to help.

On Thu, Jul 14, 2011 at 10:37 AM, Daniel Drake <dsd@laptop.org> wrote:
> Booting linux-next 20110707 on OLPC XO-1 I get:
> [    1.172947] kobject (cd8d40dc): tried to init an initialized
> object, something is seriously wrong.
> [    1.184111] Pid: 1, comm: swapper Not tainted 3.0.0-rc6-next-20110707+ #100
> [    1.198384] Call Trace:
> [    1.207741]  [<c05301a0>] ? kobject_init+0x24/0x6b
> [    1.212420]  [<c0578c49>] ? device_initialize+0x18/0x56
> [    1.227060]  [<c057960a>] ? device_register+0x8/0x10
> [    1.241052]  [<c059104a>] ? add_mtd_device+0x17a/0x1f1
> [    1.244949]  [<c0591128>] ? mtd_device_parse_register+0x67/0x7c
> [    1.259536]  [<c06c6522>] ? cafe_nand_probe+0x5c9/0x686
...
> Any ideas?

Yeah, I think it has to do with Dmitry Eremin-Solenikov's recent
changes in l2-mtd-2.6.git. Looks like the driver is trying calling
add_mtd_device() on the master MTD twice. The problem commit is (for
now):
commit 0f7451bea72c64d3f0a47850328d52f0315e2ea6
"mtd: cafe_nand.c: use mtd_device_parse_register"

Have you tried linux 3.0, which does not have the patch series that
messes with mtd partition parsing and registering?

It looks like previously, cafe_nand would always add the master
device, then it would parse and register its partitions, if found.

Since Dmitry's change, it looks like cafe_nand will add the master
device, then parse and register its partitions, if found. However, if
partitions are NOT found, then mtd_device_parse_register() falls back
to adding the master device, which was already added. In
drivers/mtd/mtdcore.c, see:

     int mtd_device_parse_register(struct mtd_info *mtd, const char **types,
     ...
     if (err > 0) {
     ...
     } else if (err == 0) {
             err = add_mtd_device(mtd);
     ...


So it looks like perhaps we can solve the problem by just killing the
"register the whole device first" and allow mtd_device_parse_register
to do it if there are no partitions. Any cafe_nand developers know if
this is a problem? i.e., is there a reason we need both the whole
device AND the partitions sent to add_mtd_device()? I'll send a full
patch with sign-off and description if there are no objections.

Brian

---
 drivers/mtd/nand/cafe_nand.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/cafe_nand.c b/drivers/mtd/nand/cafe_nand.c
index fb1425e..72d3f23 100644
--- a/drivers/mtd/nand/cafe_nand.c
+++ b/drivers/mtd/nand/cafe_nand.c
@@ -798,9 +798,6 @@ static int __devinit cafe_nand_probe(struct pci_dev *pdev,

        pci_set_drvdata(pdev, mtd);

-       /* We register the whole device first, separate from the partitions */
-       mtd_device_register(mtd, NULL, 0);
-
        mtd->name = "cafe_nand";
        mtd_device_parse_register(mtd, part_probes, 0, NULL, 0);

-- 
1.7.0.4

Re: MTD-related kobject badness during linux-next boot

[Daniel Drake]

On 2 August 2011 18:40, Brian Norris <computersforpeace@gmail.com> wrote:
> Have you tried linux 3.0, which does not have the patch series that
> messes with mtd partition parsing and registering?

Yes, and the problem does not appear there.

Daniel

Re: MTD-related kobject badness during linux-next boot

[Brian Norris]

On Tue, Aug 2, 2011 at 10:58 AM, Daniel Drake <dsd@laptop.org> wrote:
> On 2 August 2011 18:40, Brian Norris <computersforpeace@gmail.com> wrote:
>> Have you tried linux 3.0, which does not have the patch series that
>> messes with mtd partition parsing and registering?
>
> Yes, and the problem does not appear there.

As expected. Then let's wait and see if anyone has comments on my patch.

Brian

Re: MTD-related kobject badness during linux-next boot

[Jamie Iles]

Hi Brian,

On Tue, Aug 02, 2011 at 10:40:29AM -0700, Brian Norris wrote:
> Since Dmitry's change, it looks like cafe_nand will add the master
> device, then parse and register its partitions, if found. However, if
> partitions are NOT found, then mtd_device_parse_register() falls back
> to adding the master device, which was already added. In
> drivers/mtd/mtdcore.c, see:
> 
>      int mtd_device_parse_register(struct mtd_info *mtd, const char **types,
>      ...
>      if (err > 0) {
>      ...
>      } else if (err == 0) {
>              err = add_mtd_device(mtd);
>      ...
> 
> 
> So it looks like perhaps we can solve the problem by just killing the
> "register the whole device first" and allow mtd_device_parse_register
> to do it if there are no partitions. Any cafe_nand developers know if
> this is a problem? i.e., is there a reason we need both the whole
> device AND the partitions sent to add_mtd_device()? I'll send a full
> patch with sign-off and description if there are no objections.

I think that's the right thing to do.  There's actually a comment in 
drivers/mtd/mtdpart.c saying:

	/* NOTE:  we don't arrange MTDs as a tree; it'd be error-prone
         * to have the same data be in two different partitions.
         */

So I do think it should be the whole device *or* the partitions.  In any 
case, your patch looks good to me.

Jamie