[PATCH] lzma: Fix decompression speed regression

Stefan Roese sr at denx.de
Fri Jul 7 11:09:39 CEST 2023


Hi Simon,
Hi Christophe,

On 7/6/23 17:57, Simon Glass wrote:
> On Wed, 5 Jul 2023 at 09:54, Christophe Leroy
> <christophe.leroy at csgroup.eu> wrote:
>>
>> Uncompressing a 1.7Mbytes FIT image on U-boot 2023.04 takes
>> approx 7s on a powerpc 8xx.
>> The same on U-boot 2023.07-rc6 takes approx 28s unless watchdog
>> is disabled.
>>
>> During that decompression, LzmaDec_DecodeReal() calls schedule
>> 1.6 million times, that is every 4µs in average.
>>
>> In the past it used to be a call to WATCHDOG_RESET() which was
>> just calling hw_watchdog_reset().
>>
>> But the combination of commit 29caf9305b6 ("cyclic: Use schedule()
>> instead of WATCHDOG_RESET()") and commit 26e8ebcd7cb ("watchdog:
>> mpc8xxx: Make it generic") results in an heavier processing.
>>
>> However, there is absolutely no point in calling schedule() that
>> often.
>>
>> By moving and keeping only one call to schedule() in the main
>> loop the number of calls is reduced to 1.2 million which is still
>> too much. So add logic to only call schedule every 1024 times.
>> That leads to a call to schedule approx every 6ms which is still
>> far enough to entertain the watchdog which has a 1s timeout on
>> powerpc 8xx.
>>
>> powerpc 8xx being one of the slowest targets we have today in
>> U-boot, and most other watchdogs having a timeout of one minutes
>> instead of one second like the 8xx, this fix should not have
>> negative impact on other targets.
>>
>> Fixes: 29caf9305b6 ("cyclic: Use schedule() instead of WATCHDOG_RESET()")
>> Signed-off-by: Christophe Leroy <christophe.leroy at csgroup.eu>
>> ---
>>   lib/lzma/LzmaDec.c | 18 ++++--------------
>>   1 file changed, 4 insertions(+), 14 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> +Stefan Roese I wonder if we need a more general fix?

IIRC, we already have some code in the watchdog IF making sure, that the
WDT reset is not called too often. I need to re-check here. And yes, we
should probably either move this code to the common schedule() function
or add it here. Best by adding a new Kconfig option, configuring the max
schedule frequency, e.g. 1000 Hz.

Thanks,
Stefan


More information about the U-Boot mailing list