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

kernel/poll: no error happened when mutil-threads poll a same event at a same time. #33712

Closed
Zhaoningx opened this issue Mar 25, 2021 · 2 comments · Fixed by #34551
Closed
Assignees
Labels
area: Kernel bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@Zhaoningx
Copy link
Collaborator

Describe the bug
I took a look at Zephyr Specification https://docs.zephyrproject.org/latest/reference/kernel/other/polling.html#c.k_poll about kernel/poll , I think there is a test point that was not tested before, the info as below:
image
It means if there are two thread polling a same event(sem/fifo) at a same time, k_poll() immediately returns with the return value -EADDRINUSE. Is this right?
So I designed a testcase to test this, the code as below:

static K_SEM_DEFINE(test_sem, 0, 1);
static void poll_sem_t1(void *p1, void *p2, void *p3)
{
	printk("poll_sem_t1\n");

	int rc;
	struct k_poll_event *event = p1;

	rc = k_poll(event, 1, K_FOREVER);
	printk("rc1 = %d\n", rc);
	k_sem_take(&test_sem, K_FOREVER);
}

static void poll_sem_t2(void *p1, void *p2, void *p3)
{
	printk("poll_sem_t2\n");

	int rc;
	struct k_poll_event *event = p1;

	rc = k_poll(event, 1, K_FOREVER);
	printk("rc2 = %d\n", rc);
	k_sem_take(&test_sem, K_FOREVER);
}

void test_mutil_thread_poll(void)
{
	const int main_low_prio = 10;
	struct k_poll_event event;
	int old_prio = k_thread_priority_get(k_current_get());

	k_thread_priority_set(k_current_get(), main_low_prio);
	k_poll_event_init(&event, K_POLL_TYPE_SEM_AVAILABLE,
			  K_POLL_MODE_NOTIFY_ONLY, &test_sem);

	/* Spawn two threads */
	k_thread_create(&test_thread3, test_stack3,
			K_THREAD_STACK_SIZEOF(test_stack3),
			poll_sem_t1, &event, NULL, NULL, 10,
			K_INHERIT_PERMS, K_NO_WAIT);
	k_thread_create(&test_thread4, test_stack4,
			K_THREAD_STACK_SIZEOF(test_stack4),
			poll_sem_t2, &event, NULL, NULL, 10,
			K_INHERIT_PERMS, K_MSEC(10));


	k_sleep(K_MSEC(500));
	//printk("EADDRINUSE = %d\n", -EADDRINUSE);
	/* Make sem available */
	k_sem_give(&test_sem);
	k_sleep(K_MSEC(20));


	k_thread_priority_set(k_current_get(), old_prio);
}

and the output as below:

START - test_mutil_thread_poll
poll_sem_fifo_t1
poll_sem_fifo_t2
rc2 = 0
 PASS - test_mutil_thread_poll in 0.540 seconds

As you see, there is no error return value returned. So I think either the zephyr specification is wrong or there is something wrong with my code. Could you have any comments about this?

@Zhaoningx Zhaoningx added the bug The issue is a bug, or the PR is fixing a bug label Mar 25, 2021
@andyross
Copy link
Contributor

I have no idea what the documentation is trying to say there. This works, there is a list of pollers on each semaphore or fifo and as many events can wait in it as the app wants. It's true that they don't sort according to priority in the same way a wait queue does, so maybe that's what Ben was intending to enforce? I think we can fix this in documentation, though we should probably also add a warning that the waiters will be served in first-come-first-serve order and not prioritiy order.

@Zhaoningx
Copy link
Collaborator Author

Hi @andyross , yep, I totally agree with you, we can fix this in documentation, and would you like some help?

@galak galak added the priority: low Low impact/importance bug label Mar 30, 2021
@rljordan rljordan assigned Zhaoningx and unassigned andyross Apr 21, 2021
Zhaoningx added a commit to Zhaoningx/zephyr that referenced this issue Apr 29, 2021
Some description about poll are not right,
adding some new description to revise it.

Fixes zephyrproject-rtos#33712

Signed-off-by: Ningx Zhao <ningx.zhao@intel.com>
carlescufi pushed a commit that referenced this issue May 3, 2021
Some description about poll are not right,
adding some new description to revise it.

Fixes #33712

Signed-off-by: Ningx Zhao <ningx.zhao@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Kernel bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants