dell_rbtn - kernel panic at boot...

dell_rbtn - kernel panic at boot...

[Valdis Kletnieks]

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

So after I made both config variables =y, the resulting kernel built, but
died a glorious death at boot.

Dell Latitude E6530, most current released BIOS.

[    1.760739] BUG: unable to handle kernel NULL pointer dereference at 0000000000000090
[    1.760778] IP: [<ffffffffa79460cc>] klist_next+0x1c/0x1d0
[    1.760804] PGD 0 
[    1.760816] Oops: 0000 [#1] PREEMPT SMP 
[    1.760842] Modules linked in:
[    1.761268] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.1.0-rc4-next-20150521 #55
[    1.761295] Hardware name: Dell Inc. Latitude E6530/07Y85M, BIOS A16 09/24/2014
[    1.761320] task: ffff88022ca10010 ti: ffff88022ca14000 task.ti: ffff88022ca14000
[    1.761346] RIP: 0010:[<ffffffffa79460cc>]  [<ffffffffa79460cc>] klist_next+0x1c/0x1d0
[    1.761377] RSP: 0000:ffff88022ca17d58  EFLAGS: 00010292
[    1.761396] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffffa7633580
[    1.761420] RDX: 0000000000000000 RSI: ffff88022ca17db0 RDI: 0000000000000040
[    1.761445] RBP: ffff88022ca17d98 R08: 0000000000000000 R09: 00000000000a204a
[    1.761469] R10: 0000000000000000 R11: 0000000000000000 R12: ffff88022ca17db0
[    1.761471] sd 0:0:0:0: [sda] 976773168 512-byte logical blocks: (500 GB/465 GiB)
[    1.761473] sd 0:0:0:0: [sda] 4096-byte physical blocks
[    1.761533] R13: 0000000000000000 R14: ffffffffa7ff466d R15: 0000000000000000
[    1.761558] FS:  0000000000000000(0000) GS:ffff88022dc80000(0000) knlGS:0000000000000000
[    1.761585] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    1.761605] CR2: 0000000000000090 CR3: 0000000027e0b000 CR4: 00000000001406e0
[    1.761628] Stack:
[    1.761633] sd 0:0:0:0: [sda] Write Protect is off
[    1.761635] sd 0:0:0:0: [sda] Mode Sense: 00 3a 00 00
[    1.761671]  ffff88022ca17dd8 ffffffffa73b8929 0000000000000000
[    1.761694] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
[    1.761725]  0000000000000000
[    1.761743]  ffff88022ca17dfc ffffffffa7633580 ffffffffa7ff466d 0000000000000000
[    1.761782]  ffff88022ca17de8 ffffffffa74a202c ffffffffa7f07f10 0000000000000040
[    1.761846] Call Trace:
[    1.761871]  [<ffffffffa73b8929>] ? ___preempt_schedule+0x12/0x14
[    1.761894]  [<ffffffffa7633580>] ? rbtn_rfkill_set_block+0x10/0x10
[    1.761916]  [<ffffffffa7ff466d>] ? find_tokens+0xa2/0xa2
[    1.761938]  [<ffffffffa74a202c>] driver_for_each_device+0x6c/0xb0
[    1.761960]  [<ffffffffa7633752>] dell_rbtn_notifier_register+0x42/0xc0
[    1.761984]  [<ffffffffa7ff4981>] dell_init+0x314/0x98c
[    1.762003]  [<ffffffffa7ff466d>] ? find_tokens+0xa2/0xa2
[    1.762024]  [<ffffffffa70002b9>] do_one_initcall+0x99/0x210
[    1.762046]  [<ffffffffa7081300>] ? parse_args+0x140/0x3b0
[    1.762068]  [<ffffffffa7fb0237>] kernel_init_freeable+0x1c0/0x261
[    1.762090]  [<ffffffffa7946b90>] ? rest_init+0x140/0x140
[    1.762110]  [<ffffffffa7946b9e>] kernel_init+0xe/0xe0
[    1.762129]  [<ffffffffa795b00f>] ret_from_fork+0x3f/0x70
[    1.762149]  [<ffffffffa7946b90>] ? rest_init+0x140/0x140
[    1.762168] Code: ff ff 66 66 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 89 e5 41 57 41 56 41 55 41 54 49 89 fc 53 48 83 ec 18 48 8b 3f 4d 8b 6c 24 08 <4c> 8b 77 50 e8 2b 3b 01 00 4d 85 ed 0f 84 30 01 00 00 49 8b 45 
[    1.762415] RIP  [<ffffffffa79460cc>] klist_next+0x1c/0x1d0
[    1.762437]  RSP <ffff88022ca17d58>
[    1.762450] CR2: 0000000000000090
[    1.762466] ---[ end trace 24f4c0df0e74c2d0 ]---
[    1.762737] scsi 1:0:0:0: CD-ROM            MATSHITA DVD+-RW UJ8C2    1.02 PQ: 0 ANSI: 5
[    1.765506]  sda: sda1 sda2 sda3
[    1.766304] sd 0:0:0:0: [sda] Attached SCSI disk
[    1.905686] 
[    1.905686] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009
[    1.909713] Kernel Offset: 0x26000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)


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

Re: dell_rbtn - kernel panic at boot...

[Dmitry Torokhov]

On Thu, May 21, 2015 at 7:06 PM, Valdis Kletnieks
<Valdis.Kletnieks@vt.edu> wrote:
> So after I made both config variables =y, the resulting kernel built, but
> died a glorious death at boot.

I guess if both are built-in then, according to link order,
dell-laptop starts first, before dell-rbtn, and dies in
dell_rbtn_notifier_register() in call to
driver_for_each_device(&rbtn_driver.drv, ...) because rbtn_driver has
not been registered yet and thus half-initlalized.

Thanks.

-- 
Dmitry

Re: dell_rbtn - kernel panic at boot...

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 796 bytes --]

On Saturday 23 May 2015 00:53:16 Dmitry Torokhov wrote:
> On Thu, May 21, 2015 at 7:06 PM, Valdis Kletnieks
> 
> <Valdis.Kletnieks@vt.edu> wrote:
> > So after I made both config variables =y, the resulting kernel
> > built, but died a glorious death at boot.
> 
> I guess if both are built-in then, according to link order,
> dell-laptop starts first, before dell-rbtn, and dies in
> dell_rbtn_notifier_register() in call to
> driver_for_each_device(&rbtn_driver.drv, ...) because rbtn_driver has
> not been registered yet and thus half-initlalized.
> 
> Thanks.

pr_debug() messages could be useful... but no idea if we can get them.

Is there any way to fix that dependency race condition? Could 
driver_attach() function call help?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: dell_rbtn - kernel panic at boot...

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1112 bytes --]

On Saturday 23 May 2015 03:05:36 Pali Rohár wrote:
> On Saturday 23 May 2015 00:53:16 Dmitry Torokhov wrote:
> > On Thu, May 21, 2015 at 7:06 PM, Valdis Kletnieks
> > 
> > <Valdis.Kletnieks@vt.edu> wrote:
> > > So after I made both config variables =y, the resulting kernel
> > > built, but died a glorious death at boot.
> > 
> > I guess if both are built-in then, according to link order,
> > dell-laptop starts first, before dell-rbtn, and dies in
> > dell_rbtn_notifier_register() in call to
> > driver_for_each_device(&rbtn_driver.drv, ...) because rbtn_driver
> > has not been registered yet and thus half-initlalized.
> > 
> > Thanks.
> 
> pr_debug() messages could be useful... but no idea if we can get
> them.
> 
> Is there any way to fix that dependency race condition? Could
> driver_attach() function call help?

Or what about using late_initcall() instead module_init() for driver 
dell-laptop.ko? By default module_init() for static linked modules is 
macro for device_initcall() which is called before late_initcall(), 
right?

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: dell_rbtn - kernel panic at boot...

[Darren Hart]

On Sat, May 23, 2015 at 03:05:36AM +0200, Pali Rohár wrote:
> On Saturday 23 May 2015 00:53:16 Dmitry Torokhov wrote:
> > On Thu, May 21, 2015 at 7:06 PM, Valdis Kletnieks
> > 
> > <Valdis.Kletnieks@vt.edu> wrote:
> > > So after I made both config variables =y, the resulting kernel
> > > built, but died a glorious death at boot.
> > 
> > I guess if both are built-in then, according to link order,
> > dell-laptop starts first, before dell-rbtn, and dies in
> > dell_rbtn_notifier_register() in call to
> > driver_for_each_device(&rbtn_driver.drv, ...) because rbtn_driver has
> > not been registered yet and thus half-initlalized.
> > 
> > Thanks.
> 
> pr_debug() messages could be useful... but no idea if we can get them.
> 
> Is there any way to fix that dependency race condition? Could 
> driver_attach() function call help?

I believe you can avoid this by moving dell-rbtn earlier in the Makefile than
dell-laptop - but this is fragile and a hack to resolve a dependency problem.

I suppose this is why thinkpad-acpi.c is huge, and why having separate drivers
talk to eachother is discouraged.

dell-laptop seems to be the base, while dell-rbtn is the more specific of the
two - which makes dell-laptop calling a dell-rbtn function which requires
dell-rbtn to have been initialized prior a poor approach.

Greg, Matthew, I'm tempted to recommend this 434 line driver be rolled into
dell-laptop.c. Any strong opinions?

-- 
Darren Hart
Intel Open Source Technology Center

Re: dell_rbtn - kernel panic at boot...

[Matthew Garrett]

On Sun, May 24, 2015 at 09:44:32PM -0700, Darren Hart wrote:

> Greg, Matthew, I'm tempted to recommend this 434 line driver be rolled into
> dell-laptop.c. Any strong opinions?

Mrm. It's slightly conceptually nasty in that one's an ACPI driver and 
one's calling a Dell custom interface, but I think merging them is 
probably the last bad answer.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: dell_rbtn - kernel panic at boot...

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 743 bytes --]

On Monday 25 May 2015 07:01:21 Matthew Garrett wrote:
> On Sun, May 24, 2015 at 09:44:32PM -0700, Darren Hart wrote:
> > Greg, Matthew, I'm tempted to recommend this 434 line driver be
> > rolled into dell-laptop.c. Any strong opinions?
> 
> Mrm. It's slightly conceptually nasty in that one's an ACPI driver
> and one's calling a Dell custom interface, but I think merging them
> is probably the last bad answer.

I think merging does not fix our problem. dell laptop rfkill driver 
needs to be initialized after dell-rbtn acpi driver register itself.

And dell-laptop and dell-rbtn are two different devices (one dell smbios 
and one acpi) and it for me it sounds like bad idea too...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: dell_rbtn - kernel panic at boot...

[Darren Hart]

On Mon, May 25, 2015 at 08:03:42AM +0200, Pali Rohár wrote:
> On Monday 25 May 2015 07:01:21 Matthew Garrett wrote:
> > On Sun, May 24, 2015 at 09:44:32PM -0700, Darren Hart wrote:
> > > Greg, Matthew, I'm tempted to recommend this 434 line driver be
> > > rolled into dell-laptop.c. Any strong opinions?
> > 
> > Mrm. It's slightly conceptually nasty in that one's an ACPI driver
> > and one's calling a Dell custom interface, but I think merging them
> > is probably the last bad answer.
> 
> I think merging does not fix our problem. dell laptop rfkill driver 
> needs to be initialized after dell-rbtn acpi driver register itself.

If they were the same driver, you could control this ordering.

> 
> And dell-laptop and dell-rbtn are two different devices (one dell smbios 
> and one acpi) and it for me it sounds like bad idea too...

We all agree it's a bad idea - the point Mathew and I made was it may be the
"least bad" idea (all the others may be worse).

I'm looking into this, but I don't have an easy answer for you. This one is
going to take some research on your part to get to the right answer.

-- 
Darren Hart
Intel Open Source Technology Center

Re: dell_rbtn - kernel panic at boot...

[Pali Rohár]

On Tuesday 26 May 2015 21:16:58 Darren Hart wrote:
> On Mon, May 25, 2015 at 08:03:42AM +0200, Pali Rohár wrote:
> > On Monday 25 May 2015 07:01:21 Matthew Garrett wrote:
> > > On Sun, May 24, 2015 at 09:44:32PM -0700, Darren Hart wrote:
> > > > Greg, Matthew, I'm tempted to recommend this 434 line driver be
> > > > rolled into dell-laptop.c. Any strong opinions?
> > > 
> > > Mrm. It's slightly conceptually nasty in that one's an ACPI driver
> > > and one's calling a Dell custom interface, but I think merging them
> > > is probably the last bad answer.
> > 
> > I think merging does not fix our problem. dell laptop rfkill driver 
> > needs to be initialized after dell-rbtn acpi driver register itself.
> 
> If they were the same driver, you could control this ordering.
> 

Yes, I see, you are right. I can call acpi driver register function and
after that initializing dell laptop rfkill code.

> > 
> > And dell-laptop and dell-rbtn are two different devices (one dell smbios 
> > and one acpi) and it for me it sounds like bad idea too...
> 
> We all agree it's a bad idea - the point Mathew and I made was it may be the
> "least bad" idea (all the others may be worse).
> 
> I'm looking into this, but I don't have an easy answer for you. This one is
> going to take some research on your part to get to the right answer.
> 

I still think that changing module_init() could work... Do you know who
can help us with those _ini*() macros (and ideally answer how to do that)?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: dell_rbtn - kernel panic at boot...

[Darren Hart]

On Wed, May 27, 2015 at 09:28:23AM +0200, Pali Rohár wrote:
> On Tuesday 26 May 2015 21:16:58 Darren Hart wrote:
> > On Mon, May 25, 2015 at 08:03:42AM +0200, Pali Rohár wrote:
> > > On Monday 25 May 2015 07:01:21 Matthew Garrett wrote:
> > > > On Sun, May 24, 2015 at 09:44:32PM -0700, Darren Hart wrote:
> > > > > Greg, Matthew, I'm tempted to recommend this 434 line driver be
> > > > > rolled into dell-laptop.c. Any strong opinions?
> > > > 
> > > > Mrm. It's slightly conceptually nasty in that one's an ACPI driver
> > > > and one's calling a Dell custom interface, but I think merging them
> > > > is probably the last bad answer.
> > > 
> > > I think merging does not fix our problem. dell laptop rfkill driver 
> > > needs to be initialized after dell-rbtn acpi driver register itself.
> > 
> > If they were the same driver, you could control this ordering.
> > 
> 
> Yes, I see, you are right. I can call acpi driver register function and
> after that initializing dell laptop rfkill code.
> 
> > > 
> > > And dell-laptop and dell-rbtn are two different devices (one dell smbios 
> > > and one acpi) and it for me it sounds like bad idea too...
> > 
> > We all agree it's a bad idea - the point Mathew and I made was it may be the
> > "least bad" idea (all the others may be worse).
> > 
> > I'm looking into this, but I don't have an easy answer for you. This one is
> > going to take some research on your part to get to the right answer.
> > 
> 
> I still think that changing module_init() could work... Do you know who
> can help us with those _ini*() macros (and ideally answer how to do that)?

You sent the patches implementing that, I'd suggest providing complete details
on what you tested to add confidence to this working.

Greg, on Cc, is likely the best one to say if this is a reasonable approach, or
an abuse of the *_init APIs. I suspect he'll say it's the a hack to workaround a
fundamentally flawed design.

I'd suggest spending some time thinking about how this could be written such
that the individual drivers do not talk back and forth to eachother, but instead
talk to a subsystem (rfkill?) in a way that can tolerate the ordering issue.

If you simply cannot reasonably avoid the ordering issue, then I suspect the
least bad approach is to merge the drivers.

Greg - from a general driver development best practices perspective, would you
disagree with anything I've said here?

Quick summary to save Greg the search: dell-laptop calls a function in
dell-rbtn. If both are built-in, if dell-rbtn hasn't completed init yet, the
dell-laptop init will crash.

-- 
Darren Hart
Intel Open Source Technology Center

Re: dell_rbtn - kernel panic at boot...

[Pali Rohár]

On Sunday 24 May 2015 21:44:32 Darren Hart wrote:
> On Sat, May 23, 2015 at 03:05:36AM +0200, Pali Rohár wrote:
> > On Saturday 23 May 2015 00:53:16 Dmitry Torokhov wrote:
> > > On Thu, May 21, 2015 at 7:06 PM, Valdis Kletnieks
> > > 
> > > <Valdis.Kletnieks@vt.edu> wrote:
> > > > So after I made both config variables =y, the resulting kernel
> > > > built, but died a glorious death at boot.
> > > 
> > > I guess if both are built-in then, according to link order,
> > > dell-laptop starts first, before dell-rbtn, and dies in
> > > dell_rbtn_notifier_register() in call to
> > > driver_for_each_device(&rbtn_driver.drv, ...) because rbtn_driver has
> > > not been registered yet and thus half-initlalized.
> > > 
> > > Thanks.
> > 
> > pr_debug() messages could be useful... but no idea if we can get them.
> > 
> > Is there any way to fix that dependency race condition? Could 
> > driver_attach() function call help?
> 
> I believe you can avoid this by moving dell-rbtn earlier in the Makefile than
> dell-laptop - but this is fragile and a hack to resolve a dependency problem.
> 

And what about that late_initcall() instead module_init() in dell-laptop?
Will it fix this problem?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: dell_rbtn - kernel panic at boot...

[Darren Hart]

On Mon, May 25, 2015 at 04:40:14PM +0200, Pali Rohár wrote:
> On Sunday 24 May 2015 21:44:32 Darren Hart wrote:
> > On Sat, May 23, 2015 at 03:05:36AM +0200, Pali Rohár wrote:
> > > On Saturday 23 May 2015 00:53:16 Dmitry Torokhov wrote:
> > > > On Thu, May 21, 2015 at 7:06 PM, Valdis Kletnieks
> > > > 
> > > > <Valdis.Kletnieks@vt.edu> wrote:
> > > > > So after I made both config variables =y, the resulting kernel
> > > > > built, but died a glorious death at boot.
> > > > 
> > > > I guess if both are built-in then, according to link order,
> > > > dell-laptop starts first, before dell-rbtn, and dies in
> > > > dell_rbtn_notifier_register() in call to
> > > > driver_for_each_device(&rbtn_driver.drv, ...) because rbtn_driver has
> > > > not been registered yet and thus half-initlalized.
> > > > 
> > > > Thanks.
> > > 
> > > pr_debug() messages could be useful... but no idea if we can get them.
> > > 
> > > Is there any way to fix that dependency race condition? Could 
> > > driver_attach() function call help?
> > 
> > I believe you can avoid this by moving dell-rbtn earlier in the Makefile than
> > dell-laptop - but this is fragile and a hack to resolve a dependency problem.
> > 
> 
> And what about that late_initcall() instead module_init() in dell-laptop?
> Will it fix this problem?
> 

No, because late_initcall() for modules is module_init(). See
include/linux/init.h.

-- 
Darren Hart
Intel Open Source Technology Center

Re: dell_rbtn - kernel panic at boot...

[Darren Hart]

On Tue, May 26, 2015 at 09:09:30PM -0700, Darren Hart wrote:
> On Mon, May 25, 2015 at 04:40:14PM +0200, Pali Rohár wrote:
> > On Sunday 24 May 2015 21:44:32 Darren Hart wrote:
> > > On Sat, May 23, 2015 at 03:05:36AM +0200, Pali Rohár wrote:
> > > > On Saturday 23 May 2015 00:53:16 Dmitry Torokhov wrote:
> > > > > On Thu, May 21, 2015 at 7:06 PM, Valdis Kletnieks
> > > > > 
> > > > > <Valdis.Kletnieks@vt.edu> wrote:
> > > > > > So after I made both config variables =y, the resulting kernel
> > > > > > built, but died a glorious death at boot.
> > > > > 
> > > > > I guess if both are built-in then, according to link order,
> > > > > dell-laptop starts first, before dell-rbtn, and dies in
> > > > > dell_rbtn_notifier_register() in call to
> > > > > driver_for_each_device(&rbtn_driver.drv, ...) because rbtn_driver has
> > > > > not been registered yet and thus half-initlalized.
> > > > > 
> > > > > Thanks.
> > > > 
> > > > pr_debug() messages could be useful... but no idea if we can get them.
> > > > 
> > > > Is there any way to fix that dependency race condition? Could 
> > > > driver_attach() function call help?
> > > 
> > > I believe you can avoid this by moving dell-rbtn earlier in the Makefile than
> > > dell-laptop - but this is fragile and a hack to resolve a dependency problem.
> > > 
> > 
> > And what about that late_initcall() instead module_init() in dell-laptop?
> > Will it fix this problem?
> > 
> 
> No, because late_initcall() for modules is module_init(). See
> include/linux/init.h.

Apologies, in this context we're concerned about built-in, not module.

This might function as desired.

module_init() is defined as device_initcall (level 6) for built-in
late_initcall() is level 7

There is precedent for this under drivers/ - although not in anything that stood
out to me as a good exemplar. See:

b233020 Input: gpio_keys - move to late_initcall

for an example with a similar purpose.

-- 
Darren Hart
Intel Open Source Technology Center