From: Dmitry Osipenko <digetx@gmail.com>
To: "Michał Mirosław" <mirq-linux@rere.qmqm.pl>,
"Liam Girdwood" <lgirdwood@gmail.com>,
"Mark Brown" <broonie@kernel.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: simplify locking
Date: Mon, 10 Aug 2020 00:40:04 +0300 [thread overview]
Message-ID: <40871bc7-2d6c-10d4-53b3-0aded21edf3b@gmail.com> (raw)
In-Reply-To: <b22fadc413fd7a1f4018c2c9dc261abf837731cb.1597007683.git.mirq-linux@rere.qmqm.pl>
10.08.2020 00:16, Michał Mirosław пишет:
> Simplify regulator locking by removing locking around locking. rdev->ref
> is now accessed only when the lock is taken. The code still smells fishy,
> but now its obvious why.
>
> Fixes: f8702f9e4aa7 ("regulator: core: Use ww_mutex for regulators locking")
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> drivers/regulator/core.c | 37 ++++++--------------------------
> include/linux/regulator/driver.h | 1 -
> 2 files changed, 6 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 9e18997777d3..b0662927487c 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -45,7 +45,6 @@
> pr_debug("%s: " fmt, rdev_get_name(rdev), ##__VA_ARGS__)
>
> static DEFINE_WW_CLASS(regulator_ww_class);
> -static DEFINE_MUTEX(regulator_nesting_mutex);
> static DEFINE_MUTEX(regulator_list_mutex);
> static LIST_HEAD(regulator_map_list);
> static LIST_HEAD(regulator_ena_gpio_list);
> @@ -150,32 +149,13 @@ static bool regulator_ops_is_valid(struct regulator_dev *rdev, int ops)
> static inline int regulator_lock_nested(struct regulator_dev *rdev,
> struct ww_acquire_ctx *ww_ctx)
> {
> - bool lock = false;
> int ret = 0;
>
> - mutex_lock(®ulator_nesting_mutex);
> + if (ww_ctx || !mutex_trylock_recursive(&rdev->mutex.base))
Have you seen comment to the mutex_trylock_recursive()?
https://elixir.bootlin.com/linux/v5.8/source/include/linux/mutex.h#L205
* This function should not be used, _ever_. It is purely for hysterical GEM
* raisins, and once those are gone this will be removed.
I knew about this function and I don't think it's okay to use it, hence
this is why there is that "nesting_mutex" and "owner" checking.
If you disagree, then perhaps you should make another patch to remove
the stale comment to trylock_recursive().
next prev parent reply other threads:[~2020-08-09 21:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-09 21:16 [PATCH] regulator: simplify locking Michał Mirosław
2020-08-09 21:40 ` Dmitry Osipenko [this message]
2020-08-09 22:30 ` Michał Mirosław
2020-08-10 0:21 ` Dmitry Osipenko
2020-08-10 0:59 ` Michał Mirosław
2020-08-10 5:14 ` Dmitry Osipenko
2020-08-10 16:12 ` Michał Mirosław
2020-08-10 1:08 ` Michał Mirosław
2020-08-10 16:23 ` Mark Brown
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=40871bc7-2d6c-10d4-53b3-0aded21edf3b@gmail.com \
--to=digetx@gmail.com \
--cc=broonie@kernel.org \
--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;
as well as URLs for NNTP newsgroup(s).