From: Mark Brown <broonie@kernel.org>
To: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>
Cc: Dmitry Osipenko <digetx@gmail.com>,
Liam Girdwood <lgirdwood@gmail.com>,
linux-kernel@vger.kernel.org
Subject: Re: regulator: deadlock vs memory reclaim
Date: Mon, 10 Aug 2020 16:39:28 +0100 [thread overview]
Message-ID: <20200810153928.GD6438@sirena.org.uk> (raw)
In-Reply-To: <20200809222537.GA5522@qmqm.qmqm.pl>
[-- Attachment #1: Type: text/plain, Size: 1275 bytes --]
On Mon, Aug 10, 2020 at 12:25:37AM +0200, Michał Mirosław wrote:
> regulator_lock_dependent() starts by taking regulator_list_mutex, The
> same mutex covers eg. regulator initialization, including memory allocations
> that happen there. This will deadlock when you have filesystem on eg. eMMC
> (which uses a regulator to control module voltages) and you register
> a new regulator (hotplug a device?) when under memory pressure.
OK, that's very much a corner case, it only applies in the case of
coupled regulators. The most obvious thing here would be to move the
allocations on registration out of the locked region, we really only
need this in the regulator_find_coupler() call I think. If the
regulator isn't coupled we don't need to take the lock at all.
> Basically, we have a BKL for regulator_enable() and we're using ww_mutex
> as a recursive mutex with no deadlock prevention whatsoever. The locks
> also seem to cover way to much (eg. initialization even before making the
> regulator visible to the system).
Could you be more specific about what you're looking at here? There's
nothing too obvious jumping out from the code here other than the bit
around the coupling allocation, otherwise it looks like we're locking
list walks.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2020-08-10 15:40 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-09 22:25 regulator: deadlock vs memory reclaim Michał Mirosław
2020-08-10 0:09 ` Dmitry Osipenko
2020-08-10 15:39 ` Mark Brown [this message]
2020-08-10 16:09 ` Michał Mirosław
2020-08-10 17:31 ` Mark Brown
2020-08-10 19:25 ` Michał Mirosław
2020-08-10 19:41 ` Dmitry Osipenko
2020-08-10 19:51 ` Mark Brown
[not found] <cover.1597089543.git.mirq-linux@rere.qmqm.pl>
2020-08-10 20:15 ` Dmitry Osipenko
2020-08-10 20:18 ` Michał Mirosław
2020-08-10 20:21 ` Dmitry Osipenko
2020-08-10 20:56 ` Dmitry Osipenko
2020-08-10 21:23 ` Dmitry Osipenko
2020-08-11 0:07 ` Michał Mirosław
2020-08-11 15:44 ` Dmitry Osipenko
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=20200810153928.GD6438@sirena.org.uk \
--to=broonie@kernel.org \
--cc=digetx@gmail.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mirq-linux@rere.qmqm.pl \
/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