From: Chanwoo Choi <cw00.choi@samsung.com>
To: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Lukasz Luba <lukasz.luba@arm.com>
Cc: Kamil Konieczny <k.konieczny@samsung.com>,
Sylwester Nawrocki <s.nawrocki@samsung.com>,
Krzysztof Kozlowski <krzk@kernel.org>,
Willy Wolff <willy.mh.wolff.ml@gmail.com>,
MyungJoo Ham <myungjoo.ham@samsung.com>,
Kyungmin Park <kyungmin.park@samsung.com>,
Kukjin Kim <kgene@kernel.org>,
linux-pm@vger.kernel.org,
"linux-samsung-soc@vger.kernel.org"
<linux-samsung-soc@vger.kernel.org>,
linux-arm-kernel@lists.infradead.org,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: brocken devfreq simple_ondemand for Odroid XU3/4?
Date: Mon, 29 Jun 2020 10:43:39 +0900 [thread overview]
Message-ID: <691bc55c-5b04-b519-4575-6dce5ea9914c@samsung.com> (raw)
In-Reply-To: <ad4e1a73-6de3-68ee-e3b3-b30bc315bd31@samsung.com>
Hi,
Sorry for late reply because of my perfornal issue. I count not check the email.
On 6/26/20 8:22 PM, Bartlomiej Zolnierkiewicz wrote:
>
> On 6/25/20 2:12 PM, Kamil Konieczny wrote:
>> On 25.06.2020 14:02, Lukasz Luba wrote:
>>>
>>>
>>> On 6/25/20 12:30 PM, Kamil Konieczny wrote:
>>>> Hi Lukasz,
>>>>
>>>> On 25.06.2020 12:02, Lukasz Luba wrote:
>>>>> Hi Sylwester,
>>>>>
>>>>> On 6/24/20 4:11 PM, Sylwester Nawrocki wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> On 24.06.2020 12:32, Lukasz Luba wrote:
>>>>>>> I had issues with devfreq governor which wasn't called by devfreq
>>>>>>> workqueue. The old DELAYED vs DEFERRED work discussions and my patches
>>>>>>> for it [1]. If the CPU which scheduled the next work went idle, the
>>>>>>> devfreq workqueue will not be kicked and devfreq governor won't check
>>>>>>> DMC status and will not decide to decrease the frequency based on low
>>>>>>> busy_time.
>>>>>>> The same applies for going up with the frequency. They both are
>>>>>>> done by the governor but the workqueue must be scheduled periodically.
>>>>>>
>>>>>> As I have been working on resolving the video mixer IOMMU fault issue
>>>>>> described here: https://patchwork.kernel.org/patch/10861757
>>>>>> I did some investigation of the devfreq operation, mostly on Odroid U3.
>>>>>>
>>>>>> My conclusions are similar to what Lukasz says above. I would like to add
>>>>>> that broken scheduling of the performance counters read and the devfreq
>>>>>> updates seems to have one more serious implication. In each call, which
>>>>>> normally should happen periodically with fixed interval we stop the counters,
>>>>>> read counter values and start the counters again. But if period between
>>>>>> calls becomes long enough to let any of the counters overflow, we will
>>>>>> get wrong performance measurement results. My observations are that
>>>>>> the workqueue job can be suspended for several seconds and conditions for
>>>>>> the counter overflow occur sooner or later, depending among others
>>>>>> on the CPUs load.
>>>>>> Wrong bus load measurement can lead to setting too low interconnect bus
>>>>>> clock frequency and then bad things happen in peripheral devices.
>>>>>>
>>>>>> I agree the workqueue issue needs to be fixed. I have some WIP code to use
>>>>>> the performance counters overflow interrupts instead of SW polling and with
>>>>>> that the interconnect bus clock control seems to work much better.
>>>>>>
>>>>>
>>>>> Thank you for sharing your use case and investigation results. I think
>>>>> we are reaching a decent number of developers to maybe address this
>>>>> issue: 'workqueue issue needs to be fixed'.
>>>>> I have been facing this devfreq workqueue issue ~5 times in different
>>>>> platforms.
>>>>>
>>>>> Regarding the 'performance counters overflow interrupts' there is one
>>>>> thing worth to keep in mind: variable utilization and frequency.
>>>>> For example, in order to make a conclusion in algorithm deciding that
>>>>> the device should increase or decrease the frequency, we fix the period
>>>>> of observation, i.e. to 500ms. That can cause the long delay if the
>>>>> utilization of the device suddenly drops. For example we set an
>>>>> overflow threshold to value i.e. 1000 and we know that at 1000MHz
>>>>> and full utilization (100%) the counter will reach that threshold
>>>>> after 500ms (which we want, because we don't want too many interrupts
>>>>> per sec). What if suddenly utilization drops to 2% (i.e. from 5GB/s
>>>>> to 250MB/s (what if it drops to 25MB/s?!)), the counter will reach the
>>>>> threshold after 50*500ms = 25s. It is impossible just for the counters
>>>>> to predict next utilization and adjust the threshold. [...]
>>>>
>>>> irq triggers for underflow and overflow, so driver can adjust freq
>>>>
>>>
>>> Probably possible on some platforms, depends on how many PMU registers
>>> are available, what information can be can assign to them and type of
>>> interrupt. A lot of hassle and still - platform and device specific.
>>> Also, drivers should not adjust the freq, governors (different types
>>> of them with different settings that they can handle) should do it.
>>>
>>> What the framework can do is to take this responsibility and provide
>>> generic way to monitor the devices (or stop if they are suspended).
>>> That should work nicely with the governors, which try to predict the
>>> next best frequency. From my experience the more fluctuating intervals
>>> the governors are called, the more odd decisions they make.
>>> That's why I think having a predictable interval i.e. 100ms is something
>>> desirable. Tuning the governors is easier in this case, statistics
>>> are easier to trace and interpret, solution is not to platform specific,
>>> etc.
>>>
>>> Kamil do you have plans to refresh and push your next version of the
>>> workqueue solution?
>>
>> I do not, as Bartek takes over my work,
>> +CC Bartek
>
> Hi Lukasz,
>
> As you remember in January Chanwoo has proposed another idea (to allow
> selecting workqueue type by devfreq device driver):
>
> "I'm developing the RFC patch and then I'll send it as soon as possible."
> (https://lore.kernel.org/linux-pm/6107fa2b-81ad-060d-89a2-d8941ac4d17e@samsung.com/)
>
> "After posting my suggestion, we can discuss it"
> (https://lore.kernel.org/linux-pm/f5c5cd64-b72c-2802-f6ea-ab3d28483260@samsung.com/)
>
> so we have been waiting on the patch to be posted..
Sorry for this. I'll send it within few days.
>
> Similarly we have been waiting on (any) feedback for exynos-bus/nocp
> fixes for Exynos5422 support (which have been posted by Kamil also in
> January):
>
> https://lore.kernel.org/linux-pm/8f82d8d5-927b-afb4-272f-45c16b5a23b9@samsung.com/
>
> Considering the above and how hard it has been to push the changes
> through review/merge process last year we are near giving up when it
> comes to upstream devfreq contributions. Sylwester is still working on
> exynos-bus & interconnect integration (continuation of Artur Swigon's
> work from last year) & related issues (IRQ support for PPMU) but
> I'm seriously considering putting it all on-hold..
The Sylwester's patches (originally Artus Swigon's path) were reviewed
and I agreed this approach about devfreq/interconnect. It needs
the review from interconnect maintainer.
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
>
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
next prev parent reply other threads:[~2020-06-29 1:32 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-23 16:47 brocken devfreq simple_ondemand for Odroid XU3/4? Willy Wolff
2020-06-23 19:02 ` Krzysztof Kozlowski
2020-06-23 19:11 ` Krzysztof Kozlowski
2020-06-24 8:01 ` Willy Wolff
2020-06-24 8:14 ` Krzysztof Kozlowski
2020-06-24 8:52 ` Willy Wolff
2020-06-24 10:32 ` Lukasz Luba
2020-06-24 11:18 ` Kamil Konieczny
2020-06-24 12:06 ` Krzysztof Kozlowski
2020-06-24 13:03 ` Lukasz Luba
2020-06-24 13:13 ` Krzysztof Kozlowski
2020-06-24 13:42 ` Lukasz Luba
2020-06-24 15:11 ` Sylwester Nawrocki
2020-06-25 10:02 ` Lukasz Luba
2020-06-25 11:30 ` Kamil Konieczny
2020-06-25 12:02 ` Lukasz Luba
2020-06-25 12:12 ` Kamil Konieczny
2020-06-26 11:22 ` Bartlomiej Zolnierkiewicz
2020-06-29 1:43 ` Chanwoo Choi [this message]
2020-06-29 11:52 ` Lukasz Luba
2020-07-01 15:48 ` Willy Wolff
2020-06-29 11:34 ` Lukasz Luba
2020-06-26 17:50 ` Sylwester Nawrocki
2020-06-29 11:41 ` Lukasz Luba
2020-06-29 1:52 ` Chanwoo Choi
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=691bc55c-5b04-b519-4575-6dce5ea9914c@samsung.com \
--to=cw00.choi@samsung.com \
--cc=b.zolnierkie@samsung.com \
--cc=k.konieczny@samsung.com \
--cc=kgene@kernel.org \
--cc=krzk@kernel.org \
--cc=kyungmin.park@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=lukasz.luba@arm.com \
--cc=myungjoo.ham@samsung.com \
--cc=s.nawrocki@samsung.com \
--cc=willy.mh.wolff.ml@gmail.com \
/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).