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

fs/fs_epoll: add oneshot list to handle the EPOLLONESHOT correctly #8871

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

CV-Bowen
Copy link
Contributor

Summary

apache/nuttx-apps#1625 mentioned that nng can't work with epoll.
After analysis, the epoll do not handle the EPOLLONESHOT events correctly, the epoll node with events EPOLLONESHOT should be allowed to be reset by epoll_ctl(EPOLL_CTL_MOD) after notifed, but old implementation directly moved it to the free list, which is not corret.
In this PR, add a new oneshot list to solve this problem: the the epoll node with events EPOLLONESHOT will be added to the oneshot list after notified and move to setup list again after user call epoll_ctl(EPOLL_CTL_MOD) to reset it.

@jturnsek Could you take a look?

Impact

epoll

Testing

nng test in sim

Signed-off-by: wangbowen6 <wangbowen6@xiaomi.com>
@jturnsek
Copy link
Contributor

@CV-Bowen It looks ok. NNG is rearming oneshot events, so this must be it.

@CV-Bowen
Copy link
Contributor Author

CV-Bowen commented Mar 22, 2023

@CV-Bowen It looks ok. NNG is rearming oneshot events, so this must be it.

@jturnsek Thanks, @acassis Do you have some comments?

@xiaoxiang781216
Copy link
Contributor

@CV-Bowen It looks ok. NNG is rearming oneshot events, so this must be it.

@jturnsek so it's better to make epoll as the default setting

@jturnsek
Copy link
Contributor

so it's better to make epoll as the default setting

I would say no, because with small number of fds, poll should be more efficient. At least on Linux. It is the other way around when large number of fd used.

@CV-Bowen
Copy link
Contributor Author

Could you merge this PR if no more comments? @pkarashchenko

@pkarashchenko pkarashchenko merged commit 5d53c82 into apache:master Mar 23, 2023
@CV-Bowen CV-Bowen deleted the epoll branch May 27, 2023 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants