| Message ID | 20200703062622.11773-1-cw00.choi@samsung.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | PM / devfreq: Add delayed timer for polling | expand |
Hi Chanwoo,
I think it doesn't help on the benchmark I suggested that is doing only memory
accesses. With both timer, I have the same timing.
To test the benchmark with these new patches about timer:
git clone https://github.com/wwilly/benchmark.git \
&& cd benchmark \
&& source env.sh \
&& ./bench_build.sh \
&& bash source/scripts/test_dvfs_mem_patched.sh
The benchmark is set by default to run for 1s, but you can increase this by
tweaking the script as:
taskset 8 ./bench_install/bin/microbe_cache 33554431 0 9722222 <TIME in sec> ${little_freq}
Also, as I reported the issue, would it be possible to add a
Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com> ?
Many thanks in advance.
Best Regards,
Willy
Hi Chanwoo, On 7/3/20 8:26 AM, Chanwoo Choi wrote: > Add the delayed timer to devfreq framework in order to support > the periodical polling mode without stop caused by CPU idle state. Thank you, this patchset looks fine to me and is a step in the right direction: Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Some Non-CPU device must need to monitor the device status like > utilization regardless of CPU state. This is probably true for all devfreq devices using simple_ondemand governor by default: drivers/devfreq/exynos-bus.c drivers/devfreq/rk3399_dmc.c drivers/devfreq/tegra20-devfreq.c drivers/gpu/drm/lima/lima_devfreq.c drivers/gpu/drm/msm/msm_gpu.c drivers/gpu/drm/panfrost/panfrost_devfreq.c drivers/memory/samsung/exynos5422-dmc.c drivers/scsi/ufs/ufshcd.c With devfreq device polling being "coupled" to CPU idle state the devfreq subsystem behavior is completely unpredictable and unreliable. It affects both performance (device opp change up happening too late) and power consumption (device opp change down happening too late). It also causes hardware usage counters support to report too high values (because of CPU idle "coupling" the real polling period becomes larger than maximum period supported by the counter and the counter becomes fully "saturated") which negatively affects power consumption (as has been observed when using Odroid XU3/4). [ The only upside of using such "coupling" is lowered CPU power usage (in some situations) but at the (unacceptable IMHO) cost of the correctness of operations of devfreq subsystem. ] Unfortunately this patchset currently fixes only exynos5422-dmc devfreq driver. To fix problems for Exynos platforms we need to also fix exynos-bus devfreq driver. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > - patch1 explains the detailed reason why the delayed timer is required. > - patch2 initializes that exynos5422-dmc device use delayed timer as default > instead of deferrable timer. > > Chanwoo Choi (2): > PM / devfreq: Add support delayed timer for polling mode > memory: samsung: exynos5422-dmc: Use delayed timer as default > > Documentation/ABI/testing/sysfs-class-devfreq | 12 +++ > drivers/devfreq/devfreq.c | 83 ++++++++++++++++++- > drivers/memory/samsung/exynos5422-dmc.c | 1 + > include/linux/devfreq.h | 9 ++ > 4 files changed, 104 insertions(+), 1 deletion(-)
Hi all, On 7/3/20 7:26 AM, Chanwoo Choi wrote: > Add the delayed timer to devfreq framework in order to support > the periodical polling mode without stop caused by CPU idle state. > Some Non-CPU device must need to monitor the device status like > utilization regardless of CPU state. > > - patch1 explains the detailed reason why the delayed timer is required. > - patch2 initializes that exynos5422-dmc device use delayed timer as default > instead of deferrable timer. > > Chanwoo Choi (2): > PM / devfreq: Add support delayed timer for polling mode > memory: samsung: exynos5422-dmc: Use delayed timer as default > > Documentation/ABI/testing/sysfs-class-devfreq | 12 +++ > drivers/devfreq/devfreq.c | 83 ++++++++++++++++++- > drivers/memory/samsung/exynos5422-dmc.c | 1 + > include/linux/devfreq.h | 9 ++ > 4 files changed, 104 insertions(+), 1 deletion(-) > My apologizes for being late for the party. I wasn't able to run tests and I had to fix my setup after I messed up some scripts. The patch set looks good to me, so you can add my (to both patches): Reviewed-by: Lukasz Luba <lukasz.luba@arm.com> I have run these Willy's benchmark tests and I will send some follow up patches in a few minutes. Regards, Lukasz
Hi Willy, On 7/3/20 1:33 PM, Willy Wolff wrote: > Hi Chanwoo, > > I think it doesn't help on the benchmark I suggested that is doing only memory > accesses. With both timer, I have the same timing. > > To test the benchmark with these new patches about timer: > > git clone https://github.com/wwilly/benchmark.git \ > && cd benchmark \ > && source env.sh \ > && ./bench_build.sh \ > && bash source/scripts/test_dvfs_mem_patched.sh > > The benchmark is set by default to run for 1s, but you can increase this by > tweaking the script as: > > taskset 8 ./bench_install/bin/microbe_cache 33554431 0 9722222 <TIME in sec> ${little_freq} > > > Also, as I reported the issue, would it be possible to add a > Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com> ? > Many thanks in advance. Thank you for your good work and the benchmark. I hope you will continue to use it and report some issues. I am going to send a follow up patches for the DMC and I will add your 'Reported-by'. In the tests I can see the improvements, but it's worth to consult with you if I understand the new results correctly. I think there is still some area for improvements in the devfreq and you could find the interesting bits to contribute. Regards, Lukasz > > > Best Regards, > Willy >
Hi Lukasz, On 2020-07-08-15-25-03, Lukasz Luba wrote: > Hi Willy, > > On 7/3/20 1:33 PM, Willy Wolff wrote: > > Hi Chanwoo, > > > > I think it doesn't help on the benchmark I suggested that is doing only memory > > accesses. With both timer, I have the same timing. > > > > To test the benchmark with these new patches about timer: > > > > git clone https://github.com/wwilly/benchmark.git \ > > && cd benchmark \ > > && source env.sh \ > > && ./bench_build.sh \ > > && bash source/scripts/test_dvfs_mem_patched.sh > > > > The benchmark is set by default to run for 1s, but you can increase this by > > tweaking the script as: > > > > taskset 8 ./bench_install/bin/microbe_cache 33554431 0 9722222 <TIME in sec> ${little_freq} > > > > > > Also, as I reported the issue, would it be possible to add a > > Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com> ? > > Many thanks in advance. > > Thank you for your good work and the benchmark. I hope you will continue > to use it and report some issues. I am going to send a follow up patches > for the DMC and I will add your 'Reported-by'. In the tests I can see > the improvements, but it's worth to consult with you if I understand > the new results correctly. > Thanks for that. I will follow on the other patch thread discussion. > I think there is still some area for improvements in the devfreq and you > could find the interesting bits to contribute. In fact, this benchmark is motivated about part of my PhD research that has just been accepted at LCTES2020: "Performance Optimization on big.LITTLE Architectures: A Memory-latency Aware Approach" at https://dl.acm.org/doi/10.1145/3372799.3394370 Basically, it's about snooping latency with "bad" CPU DVFS choice on big.LITTLE systems or more generally SMP/AMP architecture. I'm cleaning up my code and will propose patches as an RFC later. It introduces a new CPU DVFS governor to limit snooping latency. Cheers, Willy > > Regards, > Lukasz > > > > > > > Best Regards, > > Willy > >
Hi Willy On 7/10/20 4:12 PM, Willy Wolff wrote: > Hi Lukasz, > > On 2020-07-08-15-25-03, Lukasz Luba wrote: >> Hi Willy, >> >> On 7/3/20 1:33 PM, Willy Wolff wrote: >>> Hi Chanwoo, >>> >>> I think it doesn't help on the benchmark I suggested that is doing only memory >>> accesses. With both timer, I have the same timing. >>> >>> To test the benchmark with these new patches about timer: >>> >>> git clone https://github.com/wwilly/benchmark.git \ >>> && cd benchmark \ >>> && source env.sh \ >>> && ./bench_build.sh \ >>> && bash source/scripts/test_dvfs_mem_patched.sh >>> >>> The benchmark is set by default to run for 1s, but you can increase this by >>> tweaking the script as: >>> >>> taskset 8 ./bench_install/bin/microbe_cache 33554431 0 9722222 <TIME in sec> ${little_freq} >>> >>> >>> Also, as I reported the issue, would it be possible to add a >>> Reported-by: Willy Wolff <willy.mh.wolff.ml@gmail.com> ? >>> Many thanks in advance. >> >> Thank you for your good work and the benchmark. I hope you will continue >> to use it and report some issues. I am going to send a follow up patches >> for the DMC and I will add your 'Reported-by'. In the tests I can see >> the improvements, but it's worth to consult with you if I understand >> the new results correctly. >> > > Thanks for that. I will follow on the other patch thread discussion. > >> I think there is still some area for improvements in the devfreq and you >> could find the interesting bits to contribute. > > In fact, this benchmark is motivated about part of my PhD research that has just > been accepted at LCTES2020: "Performance Optimization on big.LITTLE Architectures: > A Memory-latency Aware Approach" at https://dl.acm.org/doi/10.1145/3372799.3394370 > Congrats and thank you for the link (I will read it). > Basically, it's about snooping latency with "bad" CPU DVFS choice on big.LITTLE > systems or more generally SMP/AMP architecture. I'm cleaning up my code and will > propose patches as an RFC later. It introduces a new CPU DVFS governor to limit > snooping latency. This is interesting, please add me on CC in the patch set. Regards, Lukasz