Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 10 Aug 2020 16:39:28 +0100
From:   Mark Brown <broonie@kernel.org>
To:     =?utf-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= <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
Message-ID: <20200810153928.GD6438@sirena.org.uk>
References: <20200809222537.GA5522@qmqm.qmqm.pl>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="OaZoDhBhXzo6bW1J"
Content-Disposition: inline
In-Reply-To: <20200809222537.GA5522@qmqm.qmqm.pl>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--OaZoDhBhXzo6bW1J
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Aug 10, 2020 at 12:25:37AM +0200, Micha=C5=82 Miros=C5=82aw wrote:

> regulator_lock_dependent() starts by taking regulator_list_mutex, The
> same mutex covers eg. regulator initialization, including memory allocati=
ons
> 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.

--OaZoDhBhXzo6bW1J
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl8xajAACgkQJNaLcl1U
h9BNnQf/VnLzsKZ8MKsJKrUcITrwjL4Ty5NlAVZt14rULY5df4rpXxQ97YKGkwjD
GK0sP3nSmNHte2WUXjPfP86H+JuWk0tpOv6g06miuuFeC0bdrSZ17dchZikGM7n+
cGQHJ7x50NRtIRjjLE01h+huS61aYd/XBGZPcTpPgx8r724FoNGUWq7Ufyw0vc5f
tZFVl0F5BvDmwwgtsFBkv16VtPyDMoZp7W/oEBaECcH1myFmOMdyZ/NYQ4TfEEgt
NADH2+Z6wFJGEH4B6nkXkxAY2lY3Ya5eq8UiE9TuQru6qre59l5rC7lQTKRcRNcf
/O6+7aYsRw36Ij/2ViyNCV929Hn6tQ==
=oiI0
-----END PGP SIGNATURE-----

--OaZoDhBhXzo6bW1J--