Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests/kernel/mem_pool/mem_pool_concept/testcase.yaml#kernel.memory_pool fails on nrf52840_pca10056, nrf52_pca10040 and nrf51_pca10028 #9468

Closed
spoorthik opened this issue Aug 16, 2018 · 3 comments
Assignees
Labels
area: ARM ARM (32-bit) Architecture bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx priority: medium Medium impact/importance bug
Milestone

Comments

@spoorthik
Copy link
Contributor

Execution log:

Running test suite mpool_concept
===================================================================
starting test - test_mpool_alloc_wait_prio

    Assertion failed at /home/kspoorth/work/latest_zephyr/tests/kernel/mem_pool/mem_pool_concept/src/test_mpool_alloc_wait.c:24: tmpool_alloc_wait_timeout: k_mem_pool_alloc(&mpool1, &block, BLK_SIZE_MIN, TIMEOUT) == -EAGAIN is false

FAIL - test_mpool_alloc_wait_prio
===================================================================
starting test - test_mpool_alloc_size_roundup

    Assertion failed at /home/kspoorth/work/latest_zephyr/tests/kernel/mem_pool/mem_pool_concept/src/test_mpool_alloc_size.c:36: test_mpool_alloc_size_roundup: k_mem_pool_alloc(&mpool1, &block[i], TEST_SIZE, K_NO_WAIT) == 0 is false

FAIL - test_mpool_alloc_size_roundup
===================================================================
starting test - test_mpool_alloc_merge_failed_diff_size
PASS - test_mpool_alloc_merge_failed_diff_size
===================================================================
starting test - test_mpool_alloc_merge_failed_diff_parent

    Assertion failed at /home/kspoorth/work/latest_zephyr/tests/kernel/mem_pool/mem_pool_concept/src/test_mpool_merge_fail_diff_parent.c:33: test_mpool_alloc_merge_failed_diff_parent: k_mem_pool_alloc(&mpool1, &block[i], BLK_SIZE_MIN, K_NO_WAIT) == 0 is false

FAIL - test_mpool_alloc_merge_failed_diff_parent
===================================================================
===================================================================
PROJECT EXECUTION FAILED

Steps to reproduce:
cd tests/kernel/mem_pool/mem_pool_concept
mkdir build && cd build
cmake -DBOARD=nrf51_pca10028 .. && make flash

Latest commit: afa0e00

Platforms tested: nrf52840_pca10056, nrf52_pca10040 and nrf51_pca10028

@spoorthik spoorthik added bug The issue is a bug, or the PR is fixing a bug area: ARM ARM (32-bit) Architecture labels Aug 16, 2018
@ioannisg ioannisg added the platform: nRF Nordic nRFx label Aug 16, 2018
@nashif nashif added the priority: medium Medium impact/importance bug label Aug 23, 2018
@nashif nashif added this to the v1.13.0 milestone Aug 26, 2018
@nashif
Copy link
Member

nashif commented Aug 30, 2018

bisected to 3c7f990

commit 3c7f990367f38c84b20e7888042ea9429cfae8a6
Author: Piotr Zięcik <piotr.ziecik@nordicsemi.no>
Date:   Mon Jul 23 14:09:10 2018 +0200

    kernel: Do not use sys_clock_ticks_per_sec in _ms_to_ticks()

    The value of sys_clock_ticks_per_sec is obtained using
    simple integer division with rounding toward zero. As result
    using this variable in _ms_to_ticks() introduces some error.

    This commit eliminates sys_clock_ticks_per_sec from equation
    used in _ms_to_ticks() removing introduced error.

    Also, this commit fixes #8895.

    Signed-off-by: Piotr Zięcik <piotr.ziecik@nordicsemi.no>

@nashif
Copy link
Member

nashif commented Aug 30, 2018

@carlescufi fyi

@carlescufi
Copy link
Member

@ioannisg fyi, let's take a look at this together

carlescufi pushed a commit to carlescufi/zephyr that referenced this issue Aug 31, 2018
The following 2 improvements are contained in this patch:

- When converting from ms to ticks, instead of using hardware cycles
  per tick, use hardware cycles per second. This ensures that the
  multiplication is done before the division, increasing precision.
- When converting from ticks to ms, instead of using cycles per tick
  and cycles per sec, use ticks per sec. This too increases the
  precision.

The concept is to make the dividend as large as possible compared to the
divisor in order to lose as little precision as possible.

Fixes zephyrproject-rtos#8898
Fixes zephyrproject-rtos#9459
Fixes zephyrproject-rtos#9466
Fixes zephyrproject-rtos#9468

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
nashif pushed a commit that referenced this issue Aug 31, 2018
The following 2 improvements are contained in this patch:

- When converting from ms to ticks, instead of using hardware cycles
  per tick, use hardware cycles per second. This ensures that the
  multiplication is done before the division, increasing precision.
- When converting from ticks to ms, instead of using cycles per tick
  and cycles per sec, use ticks per sec. This too increases the
  precision.

The concept is to make the dividend as large as possible compared to the
divisor in order to lose as little precision as possible.

Fixes #8898
Fixes #9459
Fixes #9466
Fixes #9468

Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARM ARM (32-bit) Architecture bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx priority: medium Medium impact/importance bug
Projects
None yet
Development

No branches or pull requests

5 participants