[ 27.054584] the code is fine but needs lockdep annotation.
[ 27.060059] turning off the locking correctness validator

I suppose we only need some special annotation in the code?

2017/05/27 6:00 "dianders via monorail" <monorail+v2.2711505450@chromium.org

Hi #1,

this kind of issue usually caused by accessing an uninitialized lock.

checking on devfreq code, it would alloc a work in:
void devfreq_monitor_start(struct devfreq *devfreq)
{
        INIT_DEFERRABLE_WORK(&devfreq->work, devfreq_monitor);


and access it in:
void devfreq_monitor_suspend(struct devfreq *devfreq)
{
        cancel_delayed_work_sync(&devfreq->work);


but currently our dmc driver would not use this monitor, so it would not call devfreq_monitor_start to init that work.

but somehow we are trying to do call 
                devfreq_monitor_suspend(devfreq);
and
                devfreq_monitor_resume(devfreq);



so:
1/ if we do need to suspend/resume monitor work, then maybe we should start it and stop it immediately
2/ if we don't need this, maybe we should just remove these, and check would it works normally.

Hi derek,

could you help to check this?
v4.4# ag monitor drivers/devfreq/rk3399_dmc.c
365:            devfreq_monitor_suspend(devfreq);
374:            devfreq_monitor_resume(devfreq);

hmm,

looks like we need this suspend/resume to set stop_polling flag and apply suspend_freq...


so
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -344,6 +344,9 @@ static int rk3399_dfi_event_handler(struct devfreq *devfreq, unsigned int event,
                        return ret;
                }

+               devfreq_monitor_start(devfreq);
+               devfreq_monitor_stop(devfreq);
+


or remove these:
v4.4# ag monitor drivers/devfreq/rk3399_dmc.c
365:            devfreq_monitor_suspend(devfreq);
374:            devfreq_monitor_resume(devfreq);

and apply suspend_freq directly?

Doug, the log you posted says "the code is fine but needs lockdep annotation". Is it maybe missing few more lines above that we can't see in the report?

since we are already set polling_ms to 0, i think it's ok to do normal monitor start/stop...

CL:https://chromium-review.googlesource.com/517584

@5: Oops, this didn't look related, but I guess it is.

[   27.049621] INFO: trying to register non-static key.

It looks like the code printing is:

	/*
	 * Debug-check: all keys must be persistent!
 	 */
	if (!static_obj(lock->key)) {
		debug_locks_off();
		printk("INFO: trying to register non-static key.\n");
		printk("the code is fine but needs lockdep annotation.\n");
		printk("turning off the locking correctness validator.\n");
		dump_stack();

		return NULL;
	}

---

I think Jeffy's patch looks sane.  Let's see what folks who know the driver better than I think.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/b7bd9abe8337aed993cdad5f71ce8a1308517d30

commit b7bd9abe8337aed993cdad5f71ce8a1308517d30
Author: Jeffy Chen <jeffy.chen@rock-chips.com>
Date: Wed Jun 07 02:32:28 2017

CHROMIUM: rk3399: devfreq: Add missing monitor start/stop

We need to call devfreq_monitor_start before devfreq_monitor_suspend, to
avoid this lockdep warning:
[   27.054584] the code is fine but needs lockdep annotation.
[   27.060059] turning off the locking correctness validator.
[   27.065537] CPU: 4 PID: 1975 Comm: DrmThread Not tainted 4.4.64 #220
[   27.071879] Hardware name: Google Kevin (DT)
[   27.076141] Call trace:
[   27.078593] [<ffffffc000208ae4>] dump_backtrace+0x0/0x160
[   27.083982] [<ffffffc000208d74>] show_stack+0x20/0x28
[   27.089026] [<ffffffc00051a270>] dump_stack+0xb4/0xf0
[   27.094070] [<ffffffc000285674>] __lock_acquire+0x7d4/0x1960
[   27.099718] [<ffffffc0002870c8>] lock_acquire+0x23c/0x278
[   27.105108] [<ffffffc00023f858>] flush_work+0x54/0x280
[   27.110237] [<ffffffc00023fc90>] __cancel_work_timer+0x13c/0x1cc
[   27.116232] [<ffffffc00023fd74>] cancel_delayed_work_sync+0x24/0x30
[   27.122490] [<ffffffc00080cce4>] devfreq_monitor_suspend+0x74/0x80
[   27.128658] [<ffffffc00080e898>] rk3399_dfi_event_handler+0x118/0x184
[   27.135087] [<ffffffc00080aa30>] devfreq_suspend_device+0x40/0x4c

And since we are already set polling_ms to zero, adding those calls would
not affect other stuff.

BUG= chromium:726837 
TEST=build and boot

Change-Id: I17e5ab19fb073a5c64e31b1cc1940d4a7611c87e
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
Reviewed-on: https://chromium-review.googlesource.com/517584
Commit-Ready: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Derek Basehore <dbasehore@chromium.org>

[modify] https://crrev.com/b7bd9abe8337aed993cdad5f71ce8a1308517d30/drivers/devfreq/rk3399_dmc.c

Unless anyone is aware of actual problems other than the lockdep splat, we can just call this M-61.

Verified in Chrome OS 9693.1.0, 61.0.3144.0.