From: Leonard Crestez <leonard.crestez@nxp.com> To: Chanwoo Choi <cw00.choi@samsung.com>, MyungJoo Ham <myungjoo.ham@samsung.com> Cc: "Kyungmin Park" <kyungmin.park@samsung.com>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, "Matthias Kaehlcke" <mka@chromium.org>, "Artur Świgoń" <a.swigon@partner.samsung.com>, "Georgi Djakov" <georgi.djakov@linaro.org>, "Viresh Kumar" <viresh.kumar@linaro.org>, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-imx@nxp.com Subject: [PATCH 0/5] PM / devfreq: Don't take lock in devfreq_add_device Date: Thu, 14 Nov 2019 01:21:30 +0200 [thread overview] Message-ID: <cover.1573686315.git.leonard.crestez@nxp.com> (raw) Right now the devfreq_add_device takes devfreq->lock as soon as the device is allocated, this is awkward and unnecessary. In general an object should be initialized in isolation and only be made available to the system when it's in a consistent state. Locking the device during initialization causes problems (lockdep warnings) when interacting with other subsystems that also use heavy locking. There are also a few fields (such as trans_table) which are initialized after device_register even through they're readable from sysfs, these are now allocated earlier. This was spawned by the attempt to add dev_pm_qos support. I split this series here because it might benefit from separate discussion. Notifiers from pm_qos are executed under a single global dev_pm_qos_mtx and need to take devfreq->lock, this means that in order to prevent possible deadlocks all initialization of dev_pm_qos must be performed outside the devfreq->lock. PM QoS requests from sysfs should be initialized before the device is registered (because they're touched from sysfs) but all of that is currently done with devfreq->lock held! This series makes some tricky changes but the end results is easier to understand and maintain. For example it removes a scary unlock/lock pair around set_freq_table, maybe this also caused problems in the past? Alternative solutions exist: all PM QoS setup could be done after device_add just like governor setup and the min/max_freq_store could return an error if the qos request is not yet properly initialized. It might even be possible to modify dev_pm_qos to call notifiers without holding internal locks? That is generally a good idea for all notifiers. Changes since split from PM_QOS: * Add a patch which moves devm from devfreq->dev->parent to devfreq->dev. See: https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=196443 Leonard Crestez (5): PM / devfreq: Don't fail devfreq_dev_release if not in list PM / devfreq: Split device_register usage PM / devfreq: Move more initialization before registration PM / devfreq: Don't use devm on parent device PM / devfreq: Don't take lock in devfreq_add_device drivers/devfreq/devfreq.c | 41 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 29 deletions(-) -- 2.17.1
WARNING: multiple messages have this Message-ID (diff)
From: Leonard Crestez <leonard.crestez@nxp.com> To: Chanwoo Choi <cw00.choi@samsung.com>, MyungJoo Ham <myungjoo.ham@samsung.com> Cc: "Artur Świgoń" <a.swigon@partner.samsung.com>, linux-pm@vger.kernel.org, "Viresh Kumar" <viresh.kumar@linaro.org>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, "Kyungmin Park" <kyungmin.park@samsung.com>, "Matthias Kaehlcke" <mka@chromium.org>, linux-imx@nxp.com, "Georgi Djakov" <georgi.djakov@linaro.org>, linux-arm-kernel@lists.infradead.org Subject: [PATCH 0/5] PM / devfreq: Don't take lock in devfreq_add_device Date: Thu, 14 Nov 2019 01:21:30 +0200 [thread overview] Message-ID: <cover.1573686315.git.leonard.crestez@nxp.com> (raw) Right now the devfreq_add_device takes devfreq->lock as soon as the device is allocated, this is awkward and unnecessary. In general an object should be initialized in isolation and only be made available to the system when it's in a consistent state. Locking the device during initialization causes problems (lockdep warnings) when interacting with other subsystems that also use heavy locking. There are also a few fields (such as trans_table) which are initialized after device_register even through they're readable from sysfs, these are now allocated earlier. This was spawned by the attempt to add dev_pm_qos support. I split this series here because it might benefit from separate discussion. Notifiers from pm_qos are executed under a single global dev_pm_qos_mtx and need to take devfreq->lock, this means that in order to prevent possible deadlocks all initialization of dev_pm_qos must be performed outside the devfreq->lock. PM QoS requests from sysfs should be initialized before the device is registered (because they're touched from sysfs) but all of that is currently done with devfreq->lock held! This series makes some tricky changes but the end results is easier to understand and maintain. For example it removes a scary unlock/lock pair around set_freq_table, maybe this also caused problems in the past? Alternative solutions exist: all PM QoS setup could be done after device_add just like governor setup and the min/max_freq_store could return an error if the qos request is not yet properly initialized. It might even be possible to modify dev_pm_qos to call notifiers without holding internal locks? That is generally a good idea for all notifiers. Changes since split from PM_QOS: * Add a patch which moves devm from devfreq->dev->parent to devfreq->dev. See: https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=196443 Leonard Crestez (5): PM / devfreq: Don't fail devfreq_dev_release if not in list PM / devfreq: Split device_register usage PM / devfreq: Move more initialization before registration PM / devfreq: Don't use devm on parent device PM / devfreq: Don't take lock in devfreq_add_device drivers/devfreq/devfreq.c | 41 ++++++++++++--------------------------- 1 file changed, 12 insertions(+), 29 deletions(-) -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next reply other threads:[~2019-11-13 23:21 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-13 23:21 Leonard Crestez [this message] 2019-11-13 23:21 ` [PATCH 0/5] PM / devfreq: Don't take lock in devfreq_add_device Leonard Crestez 2019-11-13 23:21 ` [PATCH 1/5] PM / devfreq: Don't fail devfreq_dev_release if not in list Leonard Crestez 2019-11-13 23:21 ` Leonard Crestez 2019-12-02 1:07 ` Chanwoo Choi 2019-12-02 1:07 ` Chanwoo Choi 2019-11-13 23:21 ` [PATCH 2/5] PM / devfreq: Split device_register usage Leonard Crestez 2019-11-13 23:21 ` Leonard Crestez 2019-12-02 1:08 ` Chanwoo Choi 2019-12-02 1:08 ` Chanwoo Choi 2019-12-02 4:45 ` Leonard Crestez 2019-12-02 4:45 ` Leonard Crestez 2019-12-02 5:02 ` Chanwoo Choi 2019-12-02 5:02 ` Chanwoo Choi 2019-11-13 23:21 ` [PATCH 3/5] PM / devfreq: Move more initialization before registration Leonard Crestez 2019-11-13 23:21 ` Leonard Crestez 2019-11-13 23:21 ` [PATCH 4/5] PM / devfreq: Don't use devm on parent device Leonard Crestez 2019-11-13 23:21 ` Leonard Crestez 2019-11-13 23:21 ` [PATCH 5/5] PM / devfreq: Don't take lock in devfreq_add_device Leonard Crestez 2019-11-13 23:21 ` Leonard Crestez
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=cover.1573686315.git.leonard.crestez@nxp.com \ --to=leonard.crestez@nxp.com \ --cc=a.swigon@partner.samsung.com \ --cc=cw00.choi@samsung.com \ --cc=georgi.djakov@linaro.org \ --cc=kyungmin.park@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-imx@nxp.com \ --cc=linux-pm@vger.kernel.org \ --cc=mka@chromium.org \ --cc=myungjoo.ham@samsung.com \ --cc=rjw@rjwysocki.net \ --cc=viresh.kumar@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.