linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Runtime PM causes oops on next-20151015
@ 2015-10-15  9:22 Thierry Reding
  2015-10-15 10:04 ` Michael Ellerman
  2015-10-15 23:35 ` David Kwon (권원홍)
  0 siblings, 2 replies; 5+ messages in thread
From: Thierry Reding @ 2015-10-15  9:22 UTC (permalink / raw)
  To: Rafael J. Wysocki, Wonhong Kwon
  Cc: Len Brown, Pavel Machek, linux-next, linux-pm, linux-kernel

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

Hi Rafael, Wonhong,

Todays linux-next breaks rather spectacularly for drivers using runtime
PM. The culprit seems to be this commit:

	commit 7d24068e144adc03b805806645d732cf79488717
	Author: Wonhong Kwon <wonhongkwon@gmail.com>
	Date:   Tue Oct 6 10:10:20 2015 +0900

	    PM / hibernate: Move pm_init/pm_disk_init to late_initcall_sync

	    pm_init is being invoked by core_initcall and hibernate_image_size_init
	    calculates preferred image size (image_size) based on total pages
	    (totalram_pages). This totalram_pages can be modified during various
	    initcall-s phase and this can cause miscalculated image_size.

	    For example, when CMA is being used, init_cma_reserved_pageblock tries
	    to change the totalram_pages and this job is done during core_initcall.
	    In order words, the totalram_pages doesn't take CMA reserved pages into
	    account when image_size is calculated and it can be too small.

	    Move pm_init and pm_disk_init to late_initcall_sync so that it happens
	    after all other initcall-s change the totalram_pages.

	    Reported-by: Sangseok Lee <sangseok.lee@lge.com>
	    Signed-off-by: Wonhong Kwon <wonhong.kwon@lge.com>
	    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

I can't reply to it directly because I don't have it in any of my mail
boxes (it seems to have been sent only to the linux-pm mailing list,
even Google finds only a single match).

Here's an extract of the oops:

	[    1.395928] Unable to handle kernel NULL pointer dereference at virtual address 00000100
	[    1.404013] pgd = ffffffc000e0e000
	[    1.407417] [00000100] *pgd=000000013c007003, *pud=000000013c007003, *pmd=000000013c008003, *pte=0060000050041707
	[    1.417746] Internal error: Oops: 96000005 [#1] PREEMPT SMP
	[    1.423316] Modules linked in:
	[    1.426400] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0-rc5-next-20151015+ #1338
	[    1.434138] Hardware name: NVIDIA Tegra210 P2371 (P2180/P2597) reference board (DT)
	[    1.441789] task: ffffffc0bc0a8000 ti: ffffffc0bc084000 task.ti: ffffffc0bc084000
	[    1.449280] PC is at __queue_work+0x2c/0x240
	[    1.453551] LR is at queue_work_on+0x60/0x78
	...
	[    1.836517] Call trace:
	[    1.838968] [<ffffffc0000b4134>] __queue_work+0x2c/0x240
	[    1.844280] [<ffffffc0000b43a4>] queue_work_on+0x5c/0x78
	[    1.849599] [<ffffffc00052e508>] rpm_idle+0xc0/0x140
	[    1.854565] [<ffffffc00052e5dc>] __pm_runtime_idle+0x54/0x98
	[    1.860229] [<ffffffc00052414c>] driver_probe_device+0x164/0x2f8
	[    1.866236] [<ffffffc000524378>] __driver_attach+0x98/0xa0
	[    1.871724] [<ffffffc00052231c>] bus_for_each_dev+0x5c/0xa0
	[    1.877294] [<ffffffc000523ab4>] driver_attach+0x1c/0x28
	[    1.882608] [<ffffffc0005236e4>] bus_add_driver+0x1cc/0x238
	[    1.888180] [<ffffffc000524b1c>] driver_register+0x5c/0xf8
	[    1.893675] [<ffffffc000431800>] mipi_dsi_driver_register_full+0x50/0x60
	[    1.900374] [<ffffffc000ba1570>] panel_simple_init+0x2c/0x44
	[    1.906035] [<ffffffc000082934>] do_one_initcall+0x8c/0x1a0
	[    1.911612] [<ffffffc000b80aa8>] kernel_init_freeable+0x150/0x1f8
	[    1.917711] [<ffffffc0007f78fc>] kernel_init+0xc/0xe0

Instrumenting the code shows that pm_wq (passed to queue_work in the
rpm_idle() function) is NULL at this point. This matches up with the
change done in the above-mentioned commit, since now pm_wq only gets
initialized at late_initcall time, whereas all built-in drivers will
already be probed at device_initcall time. So I suspect that this is
going to cause crashes on a whole lot of systems (essentially every
system that tries to use runtime PM from a built-in driver).

Given the commit message I suspect that the right fix would be to split
pm_init() into two functions, one that initializes the hibernation image
and another with the PM core initialization. The pm_hibernate_init() is
probably going to work fine as late_initcall (I assume this was tested)
but the rest should probably stay at core_initcall.

I can provide a patch for the latter if everyone agrees that it's the
right thing, but in the meantime, can you please drop the above patch
from your tree to unbreak linux-next for all affected users?

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Runtime PM causes oops on next-20151015
  2015-10-15  9:22 Runtime PM causes oops on next-20151015 Thierry Reding
@ 2015-10-15 10:04 ` Michael Ellerman
  2015-10-15 13:26   ` Thierry Reding
  2015-10-15 23:35 ` David Kwon (권원홍)
  1 sibling, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2015-10-15 10:04 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Rafael J. Wysocki, Wonhong Kwon, Len Brown, Pavel Machek,
	linux-next, linux-pm, linux-kernel

On Thu, 2015-10-15 at 11:22 +0200, Thierry Reding wrote:
> Hi Rafael, Wonhong,
> 
> Todays linux-next breaks rather spectacularly for drivers using runtime
> PM. The culprit seems to be this commit:
> 
> 	commit 7d24068e144adc03b805806645d732cf79488717
> 	Author: Wonhong Kwon <wonhongkwon@gmail.com>
> 	Date:   Tue Oct 6 10:10:20 2015 +0900
> 
> 	    PM / hibernate: Move pm_init/pm_disk_init to late_initcall_sync
> 
> 	    pm_init is being invoked by core_initcall and hibernate_image_size_init
> 	    calculates preferred image size (image_size) based on total pages
> 	    (totalram_pages). This totalram_pages can be modified during various
> 	    initcall-s phase and this can cause miscalculated image_size.
> 
> 	    For example, when CMA is being used, init_cma_reserved_pageblock tries
> 	    to change the totalram_pages and this job is done during core_initcall.
> 	    In order words, the totalram_pages doesn't take CMA reserved pages into
> 	    account when image_size is calculated and it can be too small.
> 
> 	    Move pm_init and pm_disk_init to late_initcall_sync so that it happens
> 	    after all other initcall-s change the totalram_pages.
> 
> 	    Reported-by: Sangseok Lee <sangseok.lee@lge.com>
> 	    Signed-off-by: Wonhong Kwon <wonhong.kwon@lge.com>
> 	    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 

I'm seeing one too on powerpc:

Unable to handle kernel paging request for data at address 0x00000030
Faulting instruction address: 0xc0000000002e4094
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 5 PID: 1 Comm: swapper/0 Not tainted 4.3.0-rc5-next-20151015-50217-ge2be40d-dirty #37
task: c0000001fefc0000 ti: c0000001fb000000 task.ti: c0000001fb000000
NIP: c0000000002e4094 LR: c000000000c79dd4 CTR: c000000000c79d90
REGS: c0000001fb003900 TRAP: 0300   Not tainted  (4.3.0-rc5-next-20151015-50217-ge2be40d-dirty)
MSR: 8000000100009032 <SF,EE,ME,IR,DR,RI>  CR: 28000882  XER: 20000000
CFAR: c0000000000d117c DAR: 0000000000000030 DSISR: 40000000 SOFTE: 1 
GPR00: c00000000000b074 c0000001fb003b80 c000000000e93800 0000000000000000 
GPR04: c000000000d15c80 0000000000000000 00000001ff150000 0000000000000026 
GPR08: c000000000d0f748 c000000000f41a10 c000000000d0f8e8 c000000000abc59d 
GPR12: c000000000b42400 c00000000fdc1680 c00000000000b960 0000000000000000 
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
GPR24: 0000000000000000 c000000000de0360 c000000000c45d28 0000000000000000 
GPR28: c000000000deca80 c000000000d103c0 c0000001fbc905c0 c000000000d103c0 
NIP [c0000000002e4094] .sysfs_create_file_ns+0x4/0x50
LR [c000000000c79dd4] .__machine_initcall_pseries_apo_pm_init+0x44/0x60
Call Trace:
[c0000001fb003b80] [c0000000004852cc] .kasprintf+0x2c/0x40 (unreliable)
[c0000001fb003bf0] [c00000000000b074] .do_one_initcall+0xc4/0x250
[c0000001fb003ce0] [c000000000c64560] .kernel_init_freeable+0x268/0x348
[c0000001fb003db0] [c00000000000b97c] .kernel_init+0x1c/0x140
[c0000001fb003e30] [c0000000000095ac] .ret_from_kernel_thread+0x58/0xac
Instruction dump:
419e001c 3ce2ffa8 38e7be60 4bfffea4 3ce2ffa8 38e7bea8 4bfffe9c 3ce2ffa8 
38e7bea8 4bfffe8c 60000000 2c230000 <e8630030> 41820028 2fa30000 419e0020 
---[ end trace d2a742ce359651dd ]---

Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b


Which starts at:

  static int __init apo_pm_init(void)
  {
  	return (sysfs_create_file(power_kobj, &auto_poweron_attr.attr));
  }
  machine_device_initcall(pseries, apo_pm_init);

And seems to be caused by power_kobj being NULL:

  0:mon> d $power_kobj
  c000000000f41a10 0000000000000000 0000000000000000  |................|


cheers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Runtime PM causes oops on next-20151015
  2015-10-15 10:04 ` Michael Ellerman
@ 2015-10-15 13:26   ` Thierry Reding
  2015-10-15 21:24     ` Rafael J. Wysocki
  0 siblings, 1 reply; 5+ messages in thread
From: Thierry Reding @ 2015-10-15 13:26 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Rafael J. Wysocki, Wonhong Kwon, Len Brown, Pavel Machek,
	linux-next, linux-pm, linux-kernel

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

On Thu, Oct 15, 2015 at 09:04:54PM +1100, Michael Ellerman wrote:
> On Thu, 2015-10-15 at 11:22 +0200, Thierry Reding wrote:
> > Hi Rafael, Wonhong,
> > 
> > Todays linux-next breaks rather spectacularly for drivers using runtime
> > PM. The culprit seems to be this commit:
> > 
> > 	commit 7d24068e144adc03b805806645d732cf79488717
> > 	Author: Wonhong Kwon <wonhongkwon@gmail.com>
> > 	Date:   Tue Oct 6 10:10:20 2015 +0900
> > 
> > 	    PM / hibernate: Move pm_init/pm_disk_init to late_initcall_sync
> > 
> > 	    pm_init is being invoked by core_initcall and hibernate_image_size_init
> > 	    calculates preferred image size (image_size) based on total pages
> > 	    (totalram_pages). This totalram_pages can be modified during various
> > 	    initcall-s phase and this can cause miscalculated image_size.
> > 
> > 	    For example, when CMA is being used, init_cma_reserved_pageblock tries
> > 	    to change the totalram_pages and this job is done during core_initcall.
> > 	    In order words, the totalram_pages doesn't take CMA reserved pages into
> > 	    account when image_size is calculated and it can be too small.
> > 
> > 	    Move pm_init and pm_disk_init to late_initcall_sync so that it happens
> > 	    after all other initcall-s change the totalram_pages.
> > 
> > 	    Reported-by: Sangseok Lee <sangseok.lee@lge.com>
> > 	    Signed-off-by: Wonhong Kwon <wonhong.kwon@lge.com>
> > 	    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> 
> I'm seeing one too on powerpc:
> 
> Unable to handle kernel paging request for data at address 0x00000030
> Faulting instruction address: 0xc0000000002e4094
> Oops: Kernel access of bad area, sig: 11 [#1]
> SMP NR_CPUS=2048 NUMA pSeries
> Modules linked in:
> CPU: 5 PID: 1 Comm: swapper/0 Not tainted 4.3.0-rc5-next-20151015-50217-ge2be40d-dirty #37
> task: c0000001fefc0000 ti: c0000001fb000000 task.ti: c0000001fb000000
> NIP: c0000000002e4094 LR: c000000000c79dd4 CTR: c000000000c79d90
> REGS: c0000001fb003900 TRAP: 0300   Not tainted  (4.3.0-rc5-next-20151015-50217-ge2be40d-dirty)
> MSR: 8000000100009032 <SF,EE,ME,IR,DR,RI>  CR: 28000882  XER: 20000000
> CFAR: c0000000000d117c DAR: 0000000000000030 DSISR: 40000000 SOFTE: 1 
> GPR00: c00000000000b074 c0000001fb003b80 c000000000e93800 0000000000000000 
> GPR04: c000000000d15c80 0000000000000000 00000001ff150000 0000000000000026 
> GPR08: c000000000d0f748 c000000000f41a10 c000000000d0f8e8 c000000000abc59d 
> GPR12: c000000000b42400 c00000000fdc1680 c00000000000b960 0000000000000000 
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> GPR24: 0000000000000000 c000000000de0360 c000000000c45d28 0000000000000000 
> GPR28: c000000000deca80 c000000000d103c0 c0000001fbc905c0 c000000000d103c0 
> NIP [c0000000002e4094] .sysfs_create_file_ns+0x4/0x50
> LR [c000000000c79dd4] .__machine_initcall_pseries_apo_pm_init+0x44/0x60
> Call Trace:
> [c0000001fb003b80] [c0000000004852cc] .kasprintf+0x2c/0x40 (unreliable)
> [c0000001fb003bf0] [c00000000000b074] .do_one_initcall+0xc4/0x250
> [c0000001fb003ce0] [c000000000c64560] .kernel_init_freeable+0x268/0x348
> [c0000001fb003db0] [c00000000000b97c] .kernel_init+0x1c/0x140
> [c0000001fb003e30] [c0000000000095ac] .ret_from_kernel_thread+0x58/0xac
> Instruction dump:
> 419e001c 3ce2ffa8 38e7be60 4bfffea4 3ce2ffa8 38e7bea8 4bfffe9c 3ce2ffa8 
> 38e7bea8 4bfffe8c 60000000 2c230000 <e8630030> 41820028 2fa30000 419e0020 
> ---[ end trace d2a742ce359651dd ]---
> 
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> 
> 
> Which starts at:
> 
>   static int __init apo_pm_init(void)
>   {
>   	return (sysfs_create_file(power_kobj, &auto_poweron_attr.attr));
>   }
>   machine_device_initcall(pseries, apo_pm_init);
> 
> And seems to be caused by power_kobj being NULL:
> 
>   0:mon> d $power_kobj
>   c000000000f41a10 0000000000000000 0000000000000000  |................|

Yes, power_kobj is one of the other objects created by pm_init() and
which, evidently, needs to be available much earlier than late_initcall.
The patch below works for me, though I don't use hibernate on this
machine, so that part is untested.

Thierry

--- >8 ---
diff --git a/kernel/power/main.c b/kernel/power/main.c
index b2dd4d999900..2f66453e3a9b 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -648,8 +648,6 @@ static int __init pm_init(void)
 	int error = pm_start_workqueue();
 	if (error)
 		return error;
-	hibernate_image_size_init();
-	hibernate_reserved_size_init();
 	power_kobj = kobject_create_and_add("power", NULL);
 	if (!power_kobj)
 		return -ENOMEM;
@@ -661,3 +659,13 @@ static int __init pm_init(void)
 }
 
 core_initcall(pm_init);
+
+static int __init hibernate_init(void)
+{
+	hibernate_image_size_init();
+	hibernate_reserved_size_init();
+
+	return 0;
+}
+
+late_initcall_sync(hibernate_init);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: Runtime PM causes oops on next-20151015
  2015-10-15 13:26   ` Thierry Reding
@ 2015-10-15 21:24     ` Rafael J. Wysocki
  0 siblings, 0 replies; 5+ messages in thread
From: Rafael J. Wysocki @ 2015-10-15 21:24 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Michael Ellerman, Wonhong Kwon, Len Brown, Pavel Machek,
	linux-next, linux-pm, linux-kernel

On Thursday, October 15, 2015 03:26:55 PM Thierry Reding wrote:
> 
> --pf9I7BMVVzbSWLtt
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> Content-Transfer-Encoding: quoted-printable
> 
> On Thu, Oct 15, 2015 at 09:04:54PM +1100, Michael Ellerman wrote:
> > On Thu, 2015-10-15 at 11:22 +0200, Thierry Reding wrote:
> > > Hi Rafael, Wonhong,
> > >=20
> > > Todays linux-next breaks rather spectacularly for drivers using runtime
> > > PM. The culprit seems to be this commit:
> > >=20
> > > 	commit 7d24068e144adc03b805806645d732cf79488717
> > > 	Author: Wonhong Kwon <wonhongkwon@gmail.com>
> > > 	Date:   Tue Oct 6 10:10:20 2015 +0900
> > >=20
> > > 	    PM / hibernate: Move pm_init/pm_disk_init to late_initcall_sync
> > >=20
> > > 	    pm_init is being invoked by core_initcall and hibernate_image_size=
> _init
> > > 	    calculates preferred image size (image_size) based on total pages
> > > 	    (totalram_pages). This totalram_pages can be modified during vario=
> us
> > > 	    initcall-s phase and this can cause miscalculated image_size.
> > >=20
> > > 	    For example, when CMA is being used, init_cma_reserved_pageblock t=
> ries
> > > 	    to change the totalram_pages and this job is done during core_init=
> call.
> > > 	    In order words, the totalram_pages doesn't take CMA reserved pages=
>  into
> > > 	    account when image_size is calculated and it can be too small.
> > >=20
> > > 	    Move pm_init and pm_disk_init to late_initcall_sync so that it hap=
> pens
> > > 	    after all other initcall-s change the totalram_pages.
> > >=20
> > > 	    Reported-by: Sangseok Lee <sangseok.lee@lge.com>
> > > 	    Signed-off-by: Wonhong Kwon <wonhong.kwon@lge.com>
> > > 	    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >=20
> >=20
> > I'm seeing one too on powerpc:
> >=20
> > Unable to handle kernel paging request for data at address 0x00000030
> > Faulting instruction address: 0xc0000000002e4094
> > Oops: Kernel access of bad area, sig: 11 [#1]
> > SMP NR_CPUS=3D2048 NUMA pSeries
> > Modules linked in:
> > CPU: 5 PID: 1 Comm: swapper/0 Not tainted 4.3.0-rc5-next-20151015-50217-g=
> e2be40d-dirty #37
> > task: c0000001fefc0000 ti: c0000001fb000000 task.ti: c0000001fb000000
> > NIP: c0000000002e4094 LR: c000000000c79dd4 CTR: c000000000c79d90
> > REGS: c0000001fb003900 TRAP: 0300   Not tainted  (4.3.0-rc5-next-20151015=
> -50217-ge2be40d-dirty)
> > MSR: 8000000100009032 <SF,EE,ME,IR,DR,RI>  CR: 28000882  XER: 20000000
> > CFAR: c0000000000d117c DAR: 0000000000000030 DSISR: 40000000 SOFTE: 1=20
> > GPR00: c00000000000b074 c0000001fb003b80 c000000000e93800 000000000000000=
> 0=20
> > GPR04: c000000000d15c80 0000000000000000 00000001ff150000 000000000000002=
> 6=20
> > GPR08: c000000000d0f748 c000000000f41a10 c000000000d0f8e8 c000000000abc59=
> d=20
> > GPR12: c000000000b42400 c00000000fdc1680 c00000000000b960 000000000000000=
> 0=20
> > GPR16: 0000000000000000 0000000000000000 0000000000000000 000000000000000=
> 0=20
> > GPR20: 0000000000000000 0000000000000000 0000000000000000 000000000000000=
> 0=20
> > GPR24: 0000000000000000 c000000000de0360 c000000000c45d28 000000000000000=
> 0=20
> > GPR28: c000000000deca80 c000000000d103c0 c0000001fbc905c0 c000000000d103c=
> 0=20
> > NIP [c0000000002e4094] .sysfs_create_file_ns+0x4/0x50
> > LR [c000000000c79dd4] .__machine_initcall_pseries_apo_pm_init+0x44/0x60
> > Call Trace:
> > [c0000001fb003b80] [c0000000004852cc] .kasprintf+0x2c/0x40 (unreliable)
> > [c0000001fb003bf0] [c00000000000b074] .do_one_initcall+0xc4/0x250
> > [c0000001fb003ce0] [c000000000c64560] .kernel_init_freeable+0x268/0x348
> > [c0000001fb003db0] [c00000000000b97c] .kernel_init+0x1c/0x140
> > [c0000001fb003e30] [c0000000000095ac] .ret_from_kernel_thread+0x58/0xac
> > Instruction dump:
> > 419e001c 3ce2ffa8 38e7be60 4bfffea4 3ce2ffa8 38e7bea8 4bfffe9c 3ce2ffa8=
> =20
> > 38e7bea8 4bfffe8c 60000000 2c230000 <e8630030> 41820028 2fa30000 419e0020=
> =20
> > ---[ end trace d2a742ce359651dd ]---
> >=20
> > Kernel panic - not syncing: Attempted to kill init! exitcode=3D0x0000000b
> >=20
> >=20
> > Which starts at:
> >=20
> >   static int __init apo_pm_init(void)
> >   {
> >   	return (sysfs_create_file(power_kobj, &auto_poweron_attr.attr));
> >   }
> >   machine_device_initcall(pseries, apo_pm_init);
> >=20
> > And seems to be caused by power_kobj being NULL:
> >=20
> >   0:mon> d $power_kobj
> >   c000000000f41a10 0000000000000000 0000000000000000  |................|
> 
> Yes, power_kobj is one of the other objects created by pm_init() and
> which, evidently, needs to be available much earlier than late_initcall.
> The patch below works for me, though I don't use hibernate on this
> machine, so that part is untested.

Sorry about the breakage and thanks for identifying the problematic commit.

I've dropped that commit from my tree.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: Runtime PM causes oops on next-20151015
  2015-10-15  9:22 Runtime PM causes oops on next-20151015 Thierry Reding
  2015-10-15 10:04 ` Michael Ellerman
@ 2015-10-15 23:35 ` David Kwon (권원홍)
  1 sibling, 0 replies; 5+ messages in thread
From: David Kwon (권원홍) @ 2015-10-15 23:35 UTC (permalink / raw)
  To: 'Thierry Reding', 'Rafael J. Wysocki'
  Cc: 'Len Brown', 'Pavel Machek', linux-next, linux-pm,
	linux-kernel, '이건호'

Dear Thierry, 

> -----Original Message-----
> From: Thierry Reding [mailto:thierry.reding@gmail.com]
> Sent: Thursday, October 15, 2015 6:22 PM
> To: Rafael J. Wysocki; Wonhong Kwon
> Cc: Len Brown; Pavel Machek; linux-next@vger.kernel.org; linux-
> pm@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Runtime PM causes oops on next-20151015
> 
> Hi Rafael, Wonhong,
> 
> Todays linux-next breaks rather spectacularly for drivers using runtime
PM.
> The culprit seems to be this commit:
> 
> 	commit 7d24068e144adc03b805806645d732cf79488717
> 	Author: Wonhong Kwon <wonhongkwon@gmail.com>
> 	Date:   Tue Oct 6 10:10:20 2015 +0900
> 
> 	    PM / hibernate: Move pm_init/pm_disk_init to late_initcall_sync
> 
> 	    pm_init is being invoked by core_initcall and
> hibernate_image_size_init
> 	    calculates preferred image size (image_size) based on total
> pages
> 	    (totalram_pages). This totalram_pages can be modified during
> various
> 	    initcall-s phase and this can cause miscalculated image_size.
> 
> 	    For example, when CMA is being used, init_cma_reserved_pageblock
> tries
> 	    to change the totalram_pages and this job is done during
> core_initcall.
> 	    In order words, the totalram_pages doesn't take CMA reserved
> pages into
> 	    account when image_size is calculated and it can be too small.
> 
> 	    Move pm_init and pm_disk_init to late_initcall_sync so that it
> happens
> 	    after all other initcall-s change the totalram_pages.
> 
> 	    Reported-by: Sangseok Lee <sangseok.lee@lge.com>
> 	    Signed-off-by: Wonhong Kwon <wonhong.kwon@lge.com>
> 	    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> I can't reply to it directly because I don't have it in any of my mail
> boxes (it seems to have been sent only to the linux-pm mailing list, even
> Google finds only a single match).
> 
> Here's an extract of the oops:
> 
> 	[    1.395928] Unable to handle kernel NULL pointer dereference at
> virtual address 00000100
> 	[    1.404013] pgd = ffffffc000e0e000
> 	[    1.407417] [00000100] *pgd=000000013c007003,
> *pud=000000013c007003, *pmd=000000013c008003, *pte=0060000050041707
> 	[    1.417746] Internal error: Oops: 96000005 [#1] PREEMPT SMP
> 	[    1.423316] Modules linked in:
> 	[    1.426400] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0-rc5-
> next-20151015+ #1338
> 	[    1.434138] Hardware name: NVIDIA Tegra210 P2371 (P2180/P2597)
> reference board (DT)
> 	[    1.441789] task: ffffffc0bc0a8000 ti: ffffffc0bc084000 task.ti:
> ffffffc0bc084000
> 	[    1.449280] PC is at __queue_work+0x2c/0x240
> 	[    1.453551] LR is at queue_work_on+0x60/0x78
> 	...
> 	[    1.836517] Call trace:
> 	[    1.838968] [<ffffffc0000b4134>] __queue_work+0x2c/0x240
> 	[    1.844280] [<ffffffc0000b43a4>] queue_work_on+0x5c/0x78
> 	[    1.849599] [<ffffffc00052e508>] rpm_idle+0xc0/0x140
> 	[    1.854565] [<ffffffc00052e5dc>] __pm_runtime_idle+0x54/0x98
> 	[    1.860229] [<ffffffc00052414c>] driver_probe_device+0x164/0x2f8
> 	[    1.866236] [<ffffffc000524378>] __driver_attach+0x98/0xa0
> 	[    1.871724] [<ffffffc00052231c>] bus_for_each_dev+0x5c/0xa0
> 	[    1.877294] [<ffffffc000523ab4>] driver_attach+0x1c/0x28
> 	[    1.882608] [<ffffffc0005236e4>] bus_add_driver+0x1cc/0x238
> 	[    1.888180] [<ffffffc000524b1c>] driver_register+0x5c/0xf8
> 	[    1.893675] [<ffffffc000431800>]
> mipi_dsi_driver_register_full+0x50/0x60
> 	[    1.900374] [<ffffffc000ba1570>] panel_simple_init+0x2c/0x44
> 	[    1.906035] [<ffffffc000082934>] do_one_initcall+0x8c/0x1a0
> 	[    1.911612] [<ffffffc000b80aa8>] kernel_init_freeable+0x150/0x1f8
> 	[    1.917711] [<ffffffc0007f78fc>] kernel_init+0xc/0xe0
> 
> Instrumenting the code shows that pm_wq (passed to queue_work in the
> rpm_idle() function) is NULL at this point. This matches up with the
> change done in the above-mentioned commit, since now pm_wq only gets
> initialized at late_initcall time, whereas all built-in drivers will
> already be probed at device_initcall time. So I suspect that this is going
> to cause crashes on a whole lot of systems (essentially every system that
> tries to use runtime PM from a built-in driver).

I should've considered rumtime PM. Really sorry for bothering you.

> 
> Given the commit message I suspect that the right fix would be to split
> pm_init() into two functions, one that initializes the hibernation image
> and another with the PM core initialization. The pm_hibernate_init() is
> probably going to work fine as late_initcall (I assume this was tested)
> but the rest should probably stay at core_initcall.

I agree that rest of pm_init() should stay at core_initcall and only
hibernation_image_size_init() goes to late_initcall.

> 
> I can provide a patch for the latter if everyone agrees that it's the
> right thing, but in the meantime, can you please drop the above patch from
> your tree to unbreak linux-next for all affected users?
> 
> Thanks,
> Thierry

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-10-15 23:35 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-15  9:22 Runtime PM causes oops on next-20151015 Thierry Reding
2015-10-15 10:04 ` Michael Ellerman
2015-10-15 13:26   ` Thierry Reding
2015-10-15 21:24     ` Rafael J. Wysocki
2015-10-15 23:35 ` David Kwon (권원홍)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).