From: Brian Norris <briannorris@chromium.org>
To: Mark Brown <broonie@kernel.org>
Cc: Liam Girdwood <lgirdwood@gmail.com>,
Matthias Kaehlcke <mka@chromium.org>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] regulator: core: Replace _regulator_enable_delay() with fsleep()
Date: Wed, 20 Apr 2022 11:24:08 -0700 [thread overview]
Message-ID: <YmBPyJfN7we4k78/@google.com> (raw)
In-Reply-To: <YmA0vq3qkAEYGk03@sirena.org.uk>
Hi Mark,
On Wed, Apr 20, 2022 at 05:28:46PM +0100, Mark Brown wrote:
> Did the issue with the delay functions preferring delays on the higher
> end of the allowed range get fixed? That might be an issue for larger
> usleep() values.
Hmm, good question. I had a faint memory of this problem, and searching
around, I couldn't find that anybody *thought* they fixed it, and I
found evidence to the contrary (some reports complaining about, e.g.,
boot-time performance issues in drivers/usb due to the same, with no
indication that anybody truly fixed the problem).
And measurement on my systems (with expected usecs between 322 and 390)
show that we overshoot by the following stats (on >3000 samples, with
moderate load):
minimum overshoot: ~3%
maximum overshoot: ~150%
median overshoot: ~104%
mean overshoot: ~98%
stddev: 0.207
I guess this is one aspect that Documentation/timers/timers-howto.rst is
referring to, when it says: "Exact tolerances here are very situation
specific, thus it is left to the caller to determine a reasonable
range." It feels like fsleep()'s "x2" is pretty arbitrary and often not
what people want, but maybe it's good enough for non-sensitive cases.
So maybe it's better to retain the regulator core helper
(_regulator_enable_delay()) and rename/repurpose it for my patch 1?
I feel like there's some room for improvement in either fsleep() or
usleep_range() or both, but I'm not sure exactly how to go about that
right now.
Brian
next prev parent reply other threads:[~2022-04-20 18:24 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-18 21:12 [PATCH 1/2] regulator: core: Sleep (not delay) in set_voltage() Brian Norris
2022-04-18 21:12 ` [PATCH 2/2] regulator: core: Replace _regulator_enable_delay() with fsleep() Brian Norris
2022-04-18 22:53 ` Matthias Kaehlcke
2022-04-20 16:28 ` Mark Brown
2022-04-20 18:24 ` Brian Norris [this message]
2022-04-20 21:11 ` Mark Brown
2022-04-18 22:50 ` [PATCH 1/2] regulator: core: Sleep (not delay) in set_voltage() Matthias Kaehlcke
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=YmBPyJfN7we4k78/@google.com \
--to=briannorris@chromium.org \
--cc=broonie@kernel.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mka@chromium.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