linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


             reply	other threads:[~2019-11-13 23:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-13 23:21 Leonard Crestez [this message]
2019-11-13 23:21 ` [PATCH 1/5] PM / devfreq: Don't fail devfreq_dev_release if not in list Leonard Crestez
2019-12-02  1:07   ` Chanwoo Choi
2019-11-13 23:21 ` [PATCH 2/5] PM / devfreq: Split device_register usage Leonard Crestez
2019-12-02  1:08   ` Chanwoo Choi
2019-12-02  4:45     ` Leonard Crestez
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 ` [PATCH 4/5] PM / devfreq: Don't use devm on parent device Leonard Crestez
2019-11-13 23:21 ` [PATCH 5/5] PM / devfreq: Don't take lock in devfreq_add_device 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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).