| Message ID | 20240611-docker-image-v1-12-51472eb70357@flygoat.com |
|---|---|
| State | New |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | New CI image and fixes | expand |
On Tue, Jun 11, 2024 at 10:04:11PM +0100, Jiaxun Yang wrote: > The default value CYCLIC_MAX_CPU_TIME_US was 1000, which is > a little bit too low for slower hardware and sandbox. > > On my MIPS Boston FPGA board with interaptiv CPU, wdt_cyclic > can easily take 3200 us to run. > > On azure pipeline sandbox_clang, wdt_cyclic some times goes > beyond 1300 us. > > Raise default value to 5000, which is the value already taken > by octeon_nic32. This is still sufficent to maintain system > responsiveness. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > --- > common/Kconfig | 2 +- > configs/octeon_nic23_defconfig | 1 - > 2 files changed, 1 insertion(+), 2 deletions(-) This seems similar to: https://patchwork.ozlabs.org/project/uboot/patch/20240524210817.1953298-1-rasmus.villemoes@prevas.dk/ at least for CI. And for outside CI, I'm OK with just having the value be changed in the defconfig as needed. We do support using config fragments, so keeping such changes locally isn't too hard.
在2024年6月12日六月 下午5:00，Tom Rini写道： [...] >> configs/octeon_nic23_defconfig | 1 - >> 2 files changed, 1 insertion(+), 2 deletions(-) > > This seems similar to: > https://patchwork.ozlabs.org/project/uboot/patch/20240524210817.1953298-1-rasmus.villemoes@prevas.dk/ > > at least for CI. And for outside CI, I'm OK with just having the value > be changed in the defconfig as needed. We do support using config > fragments, so keeping such changes locally isn't too hard. So the default value is a little bit too hard even for some of the actual hardware. I think being permissive here can prevent people hit into the problem only after enabling cyclic on their own board. Thanks > > -- > Tom > > 附件: > * signature.asc
On Wed, Jun 12, 2024 at 05:13:37PM +0100, Jiaxun Yang wrote: > > > 在2024年6月12日六月 下午5:00，Tom Rini写道： > [...] > >> configs/octeon_nic23_defconfig | 1 - > >> 2 files changed, 1 insertion(+), 2 deletions(-) > > > > This seems similar to: > > https://patchwork.ozlabs.org/project/uboot/patch/20240524210817.1953298-1-rasmus.villemoes@prevas.dk/ > > > > at least for CI. And for outside CI, I'm OK with just having the value > > be changed in the defconfig as needed. We do support using config > > fragments, so keeping such changes locally isn't too hard. > > So the default value is a little bit too hard even for some of the actual > hardware. Right, there's some platforms where it's too small and we should just bump it up. I think for now the default is what we want it to be for most platforms.
On 6/12/24 18:50, Tom Rini wrote: > On Wed, Jun 12, 2024 at 05:13:37PM +0100, Jiaxun Yang wrote: >> >> >> 在2024年6月12日六月 下午5:00，Tom Rini写道： >> [...] >>>> configs/octeon_nic23_defconfig | 1 - >>>> 2 files changed, 1 insertion(+), 2 deletions(-) >>> >>> This seems similar to: >>> https://patchwork.ozlabs.org/project/uboot/patch/20240524210817.1953298-1-rasmus.villemoes@prevas.dk/ >>> >>> at least for CI. And for outside CI, I'm OK with just having the value >>> be changed in the defconfig as needed. We do support using config >>> fragments, so keeping such changes locally isn't too hard. >> >> So the default value is a little bit too hard even for some of the actual >> hardware. > > Right, there's some platforms where it's too small and we should just > bump it up. I think for now the default is what we want it to be for > most platforms. The current default value is definitely too small, especially when CI is involved (I did not have this in mind when implementing), so: Acked-by: Stefan Roese <sr@denx.de> Thanks, Stefan
On Fri, Jun 14, 2024 at 04:13:54PM +0200, Stefan Roese wrote: > On 6/12/24 18:50, Tom Rini wrote: > > On Wed, Jun 12, 2024 at 05:13:37PM +0100, Jiaxun Yang wrote: > > > > > > > > > 在2024年6月12日六月 下午5:00，Tom Rini写道： > > > [...] > > > > > configs/octeon_nic23_defconfig | 1 - > > > > > 2 files changed, 1 insertion(+), 2 deletions(-) > > > > > > > > This seems similar to: > > > > https://patchwork.ozlabs.org/project/uboot/patch/20240524210817.1953298-1-rasmus.villemoes@prevas.dk/ > > > > > > > > at least for CI. And for outside CI, I'm OK with just having the value > > > > be changed in the defconfig as needed. We do support using config > > > > fragments, so keeping such changes locally isn't too hard. > > > > > > So the default value is a little bit too hard even for some of the actual > > > hardware. > > > > Right, there's some platforms where it's too small and we should just > > bump it up. I think for now the default is what we want it to be for > > most platforms. > > The current default value is definitely too small, especially when CI > is involved (I did not have this in mind when implementing), so: > > Acked-by: Stefan Roese <sr@denx.de> Can we please get either this, or https://patchwork.ozlabs.org/project/uboot/patch/20240524210817.1953298-1-rasmus.villemoes@prevas.dk/ merged for master? The number of false negatives in CI due to this is big issue for getting more contributors to use CI. Thanks.
diff --git a/common/Kconfig b/common/Kconfig index 5e3070e92539..4bb9f08977aa 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -628,7 +628,7 @@ if CYCLIC config CYCLIC_MAX_CPU_TIME_US int "Sets the max allowed time for a cyclic function in us" - default 1000 + default 5000 help The max allowed time for a cyclic function in us. If a functions takes longer than this duration this function will get unregistered diff --git a/configs/octeon_nic23_defconfig b/configs/octeon_nic23_defconfig index f7c35536a021..5a8db5a0876b 100644 --- a/configs/octeon_nic23_defconfig +++ b/configs/octeon_nic23_defconfig @@ -25,7 +25,6 @@ CONFIG_SYS_PBSIZE=276 CONFIG_SYS_CONSOLE_ENV_OVERWRITE=y # CONFIG_SYS_DEVICE_NULLDEV is not set CONFIG_CYCLIC=y -CONFIG_CYCLIC_MAX_CPU_TIME_US=5000 CONFIG_ARCH_MISC_INIT=y CONFIG_BOARD_EARLY_INIT_F=y CONFIG_BOARD_LATE_INIT=y
The default value CYCLIC_MAX_CPU_TIME_US was 1000, which is a little bit too low for slower hardware and sandbox. On my MIPS Boston FPGA board with interaptiv CPU, wdt_cyclic can easily take 3200 us to run. On azure pipeline sandbox_clang, wdt_cyclic some times goes beyond 1300 us. Raise default value to 5000, which is the value already taken by octeon_nic32. This is still sufficent to maintain system responsiveness. Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> --- common/Kconfig | 2 +- configs/octeon_nic23_defconfig | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)