linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Willy Wolff <willy.mh.wolff.ml@gmail.com>
To: Lukasz Luba <lukasz.luba@arm.com>
Cc: "linux-samsung-soc@vger.kernel.org"
	<linux-samsung-soc@vger.kernel.org>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Kamil Konieczny <k.konieczny@samsung.com>,
	linux-pm@vger.kernel.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	MyungJoo Ham <myungjoo.ham@samsung.com>,
	Kukjin Kim <kgene@kernel.org>,
	Sylwester Nawrocki <s.nawrocki@samsung.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: brocken devfreq simple_ondemand for Odroid XU3/4?
Date: Wed, 1 Jul 2020 17:48:04 +0200	[thread overview]
Message-ID: <20200701154804.f4amjgnqmprcfonw@macmini.local> (raw)
In-Reply-To: <be215777-54fd-ed84-0709-1d276bc3fe90@arm.com>

On 2020-06-29-12-52-10, Lukasz Luba wrote:
> Hi Chanwoo,
> 
> On 6/29/20 2:43 AM, Chanwoo Choi wrote:
> > Hi,
> > 
> > Sorry for late reply because of my perfornal issue. I count not check the email.
> 
> I hope you are good now.
> 
> > 
> > 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.
> 
> 
> Feel free to add me on CC, I can review&test the patches if you like.

Please CC me too.

> 
> Stay safe and healthy.
> 
> Regards,
> Lukasz
> 
Cheers,
Willy

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-07-01 15:49 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
2020-06-29 11:52                     ` Lukasz Luba
2020-07-01 15:48                       ` Willy Wolff [this message]
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=20200701154804.f4amjgnqmprcfonw@macmini.local \
    --to=willy.mh.wolff.ml@gmail.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=cw00.choi@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 \
    /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).