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@xxxxxxxxxxx/) > > "After posting my suggestion, we can discuss it" > (https://lore.kernel.org/linux-pm/f5c5cd64-b72c-2802-f6ea-ab3d28483260@xxxxxxxxxxx/) > > 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@xxxxxxxxxxx/ > > 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
