diff mbox series

Performance: devfreq: avoid devfreq delay work re-init before queue, which will cause delayed work queue reentry.

Message ID 20231104051226.6249-1-huangzaiyang@oppo.com (mailing list archive)
State New
Delegated to: Chanwoo Choi
Headers show
Series Performance: devfreq: avoid devfreq delay work re-init before queue, which will cause delayed work queue reentry. | expand

Commit Message

黄再漾(Joyyoung Huang) Nov. 4, 2023, 5:12 a.m. UTC 
From: huangzaiyang <joyyoung.wang@gmail.com>

There is a timer_list race condition when executing the following test shell script:
'''
#!/vendor/bin/sh
while true
do
        echo "simple_ondemand" > /sys/class/devfreq/1d84000.ufshc/governor
        echo "performance" > /sys/class/devfreq/1d84000.ufshc/governor
done
'''

[13511.214366][    C3] Unable to handle kernel paging request at virtual address dead00000000012a
[13511.214393][    C3] Mem abort info:
[13511.214398][    C3]   ESR = 0x96000044
[13511.214404][    C3]   EC = 0x25: DABT (current EL), IL = 32 bits
[13511.214409][    C3]   SET = 0, FnV = 0
[13511.214414][    C3]   EA = 0, S1PTW = 0
[13511.214417][    C3] Data abort info:
[13511.214422][    C3]   ISV = 0, ISS = 0x00000044
[13511.214427][    C3]   CM = 0, WnR = 1
[13511.214432][    C3] [dead00000000012a] address between user and kernel address ranges
[13511.214439][    C3] Internal error: Oops: 96000044 [#1] PREEMPT SMP
[13511.215449][    C3] CPU: 3 PID: 0 Comm: swapper/3 Tainted: G S      W  O      5.10.168-android12-9-o-g63cc297a7b34 #1
[13511.215454][    C3] Hardware name: Qualcomm Technologies, Inc. Cape MTP, Whiteswan (DT)
[13511.215460][    C3] pstate: 82400085 (Nzcv daIf +PAN -UAO +TCO BTYPE=--)
[13511.215472][    C3] pc : expire_timers+0x9c/0x428
[13511.215478][    C3] lr : __run_timers+0x1f0/0x328
[13511.215483][    C3] sp : ffffffc00801bdd0
[13511.215487][    C3] x29: ffffffc00801bdf0 x28: ffffffdb87b31698
[13511.215493][    C3] x27: ffffffdb87999e58 x26: ffffffdb87966008
[13511.215499][    C3] x25: 0000000000000001 x24: ffffff8001734a00
[13511.215506][    C3] x23: 00000000000000e0 x22: dead000000000122
[13511.215512][    C3] x21: 000000010032658e x20: ffffff89f7a9ae80
[13511.215518][    C3] x19: ffffffc00801be50 x18: ffffffc00801d038
[13511.215525][    C3] x17: 0000000000000240 x16: 0000000000000201
[13511.215532][    C3] x15: ffffffffffffffff x14: ffffff89f7a9aef8
[13511.215538][    C3] x13: 0000000000000240 x12: ffffff89f7a9aea8
[13511.215544][    C3] x11: 0000000000000021 x10: 000000014032658e
[13511.215550][    C3] x9 : ffffffc00801be50 x8 : dead000000000122
[13511.215556][    C3] x7 : ffff71646c68735e x6 : ffffff89f7aaae58
[13511.215563][    C3] x5 : 0000000000000000 x4 : 0000000000000101
[13511.215569][    C3] x3 : ffffff89f7a9aef0 x2 : ffffff89f7a9aef0
[13511.215575][    C3] x1 : ffffffc00801be50 x0 : ffffff8045804428
[13511.215581][    C3] Call trace:
[13511.215586][    C3]  expire_timers+0x9c/0x428
[13511.215591][    C3]  __run_timers+0x1f0/0x328
[13511.215596][    C3]  run_timer_softirq+0x28/0x58
[13511.215602][    C3]  efi_header_end+0x168/0x5ec
[13511.215610][    C3]  __irq_exit_rcu+0x108/0x124
[13511.215617][    C3]  __handle_domain_irq+0x118/0x1e4
[13511.215625][    C3]  gic_handle_irq.31230+0x6c/0x250
[13511.215630][    C3]  el1_irq+0xe4/0x1c0
[13511.215638][    C3]  cpuidle_enter_state+0x3a4/0xa04
[13511.215644][    C3]  do_idle+0x308/0x574
[13511.215649][    C3]  cpu_startup_entry+0x84/0x90
[13511.215656][    C3]  secondary_start_kernel+0x204/0x274
[13511.215664][    C3] Code: d503201f a9402408 f9000128 b4000048 (f9000509)
[13511.215670][    C3] ---[ end trace 5100bad72a35d566 ]---
[13511.215676][    C3] Kernel panic - not syncing: Oops: Fatal exception in interrupt

This is because when switching the governor through the sys node,
the devfreq_monitor_start function will re-initialize the delayed work task,
which will cause the delay work pending flag to become invalid, and the timer pending judgment contained in the delayed work will also become invalid,
and then the pending interception will be executed when the queue is executed.

So we remove the delay work'initialization work to the devfreq_add_device and timer_store functions,
and the delay work pending judgment is performed before the devfreq_monitor_start function performs the queue operation.

Signed-off-by: ZaiYang Huang <hzy0719@163.com>
---
 drivers/devfreq/devfreq.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

--
2.17.1
diff mbox series

Patch

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index b3a68d5833bd..ce4e1bbb6fea 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -480,21 +480,10 @@  static void devfreq_monitor(struct work_struct *work)
  */
 void devfreq_monitor_start(struct devfreq *devfreq)
 {
-       if (IS_SUPPORTED_FLAG(devfreq->governor->flags, IRQ_DRIVEN))
+       if (devfreq->governor->interrupt_driven)
                return;

-       switch (devfreq->profile->timer) {
-       case DEVFREQ_TIMER_DEFERRABLE:
-               INIT_DEFERRABLE_WORK(&devfreq->work, devfreq_monitor);
-               break;
-       case DEVFREQ_TIMER_DELAYED:
-               INIT_DELAYED_WORK(&devfreq->work, devfreq_monitor);
-               break;
-       default:
-               return;
-       }
-
-       if (devfreq->profile->polling_ms)
+       if (devfreq->profile->polling_ms && !delayed_work_pending(&devfreq->work))
                queue_delayed_work(devfreq_wq, &devfreq->work,
                        msecs_to_jiffies(devfreq->profile->polling_ms));
 }
@@ -830,6 +819,17 @@  struct devfreq *devfreq_add_device(struct device *dev,
                goto err_dev;
        }

+       switch (devfreq->profile->timer) {
+       case DEVFREQ_TIMER_DEFERRABLE:
+               INIT_DEFERRABLE_WORK(&devfreq->work, devfreq_monitor);
+               break;
+       case DEVFREQ_TIMER_DELAYED:
+               INIT_DELAYED_WORK(&devfreq->work, devfreq_monitor);
+               break;
+       default:
+               dev_err(dev, "%s: Target devfreq(%s)'s profile timer has no settings \n", devfreq->governor_name,
+                       __func__);
+       }
        if (!devfreq->profile->max_state || !devfreq->profile->freq_table) {
                mutex_unlock(&devfreq->lock);
                err = set_freq_table(devfreq);
@@ -1860,6 +1860,18 @@  static ssize_t timer_store(struct device *dev, struct device_attribute *attr,
        df->profile->timer = timer;
        mutex_unlock(&df->lock);

+       switch (df->profile->timer) {
+       case DEVFREQ_TIMER_DEFERRABLE:
+               INIT_DEFERRABLE_WORK(&df->work, devfreq_monitor);
+               break;
+       case DEVFREQ_TIMER_DELAYED:
+               INIT_DELAYED_WORK(&df->work, devfreq_monitor);
+               break;
+       default:
+               dev_err(dev, "%s: Target devfreq(%s)'s profile timer has no settings \n", df->governor_name,
+                       __func__);
+       }
+
        ret = df->governor->event_handler(df, DEVFREQ_GOV_STOP, NULL);
        if (ret) {
                dev_warn(dev, "%s: Governor %s not stopped(%d)\n",