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/interrupt failed on ARC #23476

Closed
chen-png opened this issue Mar 16, 2020 · 3 comments · Fixed by #24445
Closed

tests/kernel/interrupt failed on ARC #23476

chen-png opened this issue Mar 16, 2020 · 3 comments · Fixed by #24445
Assignees
Labels
area: ARC ARC Architecture area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@chen-png
Copy link
Collaborator

chen-png commented Mar 16, 2020

Describe the bug
Running tests/kernel/interrupt on iotdk board, it will stop at "test_prevent_interruption" and fail due to timeout.

To Reproduce
Steps to reproduce the behavior:

  1. sanitycheck -p iotdk --device-testing --device-serial /dev/ttyUSB0 -T tests/kernel/interrupt/
  2. See error in hander.log file.

Screenshots or console output

*** Booting Zephyr OS build zephyr-v2.2.0-368-g22b9167acb52  ***
Running test suite interrupt_feature

starting test - test_isr_dynamic
PASS - test_isr_dynamic

starting test - test_nested_isr
Triggering irq : 93
isr0 running !!
Triggering irq : 92
isr1 ran !!
isr0 execution completed !!
PASS - test_nested_isr

**starting test - test_prevent_interruption
locking interrupts**

Environment (please complete the following information):

  • OS: fedora 28
  • Toolchain: zephyr-sdk-0.11.1
  • Commit ID: 22b9167
@chen-png chen-png added the bug The issue is a bug, or the PR is fixing a bug label Mar 16, 2020
@nashif nashif added the priority: medium Medium impact/importance bug label Apr 7, 2020
@andrewboie
Copy link
Contributor

This also fails on nsim_sem, nsim_em, nsim_sem_mpu_stack_guard, nsim_em_mpu_stack_guard

@andrewboie andrewboie changed the title tests/kernel/interrupt failed on iotdk board. tests/kernel/interrupt failed on ARC Apr 10, 2020
@vonhust
Copy link

vonhust commented Apr 13, 2020

@andrewboie @ruuddw @abrodkin , here is my investigation
In the following codes of prevent_irq.c

         key = irq_lock();
	handler_result = 0;

	k_timer_init(&irqlock_timer, timer_handler, NULL);

	/* Start the timer and busy-wait for a bit with IRQs locked. The
	 * timer ought to have fired during this time if interrupts weren't
	 * locked -- but since they are, check_lock_new isn't updated.
	 */
	k_timer_start(&irqlock_timer, K_MSEC(DURATION), K_NO_WAIT);
	k_busy_wait(MS_TO_US(1000));
	zassert_not_equal(handler_result, HANDLER_TOKEN,
		"timer interrupt was serviced while interrupts are locked");

	printk("unlocking interrupts\n");
	irq_unlock(key);

systick timer irq cannot be handled as irq is locked. So it means the sys tick may be lost.

Meanwhile, k_busy_wait(MS_TO_US(1000)) relies on k_cycle_get_32, then to arch_k_cycle_get_32()->z_timer_cycle_get_32

However, as the sys tick is not updated because of irq lock , the value return by z_timer_cycle_get_32 will wrap back. ARC's timer is a free running timer and will wrap to 0 when the counter reaches the LIMIT. (other arch like ARM has similar issue)

This will cause processor loops forever in k_busy_wait.

The root issue is how to maintain the sys cycles when sys tick irq is not updated and there is no global wall clock.

For the test, a quick fix is don't k_busy_wait too long when irq is locked. e.g,
k_busy_wait(MS_TO_US(1000)); -> k_busy_wait(MS_TO_US(K_MSEC(2 * DURATION)));

a second fix is to implement CONFIG_ARCH_HAS_CUSTOM_BUSY_WAIT

a third fix is to maintain the correct wall clock in arch_k_cycle_get_32/z_timer_cycle_get32.

I think in high-level, it needs some discussion about this.

@ruuddw
Copy link
Member

ruuddw commented Apr 15, 2020

It could be argued that busy waiting for longer than 2 (dynamic) system ticks is a-typical use ("A busy wait is typically used instead of thread sleeping when the required delay is too short to warrant having the scheduler context switch from the current thread to another thread and then back again.").
However, I think this is more than just a test issue, unless k_busy_wait() gets a pre-condition that (timer) interrupts should be enabled when calling -> I don't like the first proposed fix. I think an ARC custom busy wait is the better solution (e.g. calculate and count the number of required timer wraps, and/or test if the busy wait time exceeds the currently set timer alarm).

@carlescufi carlescufi added area: Tests Issues related to a particular existing or missing test and removed area: Testing labels Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ARC ARC Architecture area: Tests Issues related to a particular existing or missing test bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
6 participants