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

Switch synchronous thread to thread_local storage #3330

Merged
merged 3 commits into from
Mar 6, 2024

Conversation

Alan-Jowett
Copy link
Member

Description

IOCTLs sent via a file handle that is opened without the FILE_FLAG_OVERLAPPED flag are serialized in the file object. This will affect performance if multiple user mode threads are making concurrent IOCTL requests.

This change switches the synchronous handle to being per-thread, with the handle being closed on thread detach.

Testing

CI/CD

Documentation

No.

Installation

No.

@Alan-Jowett
Copy link
Member Author

Need to investigate failures.

@Alan-Jowett Alan-Jowett marked this pull request as ready for review March 5, 2024 23:37
@Alan-Jowett
Copy link
Member Author

Isusue resolved, ready for review.

@matthewige
Copy link
Collaborator

Code itself looks good to me, but do we have existing tests which utilize ebpf api on multiple threads?

@Alan-Jowett
Copy link
Member Author

Code itself looks good to me, but do we have existing tests which utilize ebpf api on multiple threads?

Added dedicates IOCTL stress test, FWIW.

}
}

// Wait for 10 seconds
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment is out of date.

@Alan-Jowett Alan-Jowett added this pull request to the merge queue Mar 6, 2024
Merged via the queue into microsoft:main with commit f7bf6d0 Mar 6, 2024
80 checks passed
@Alan-Jowett Alan-Jowett deleted the handle_per_thread branch March 6, 2024 17:34
shankarseal pushed a commit to shankarseal/ebpf-for-windows that referenced this pull request Mar 22, 2024
* Switch synchronous thread to thread_local storage

Signed-off-by: Alan Jowett <[email protected]>

* Add dedicated calls to cleanup device handles on thread attach/detach

Signed-off-by: Alan Jowett <[email protected]>

* Add dedicate test to stress device handling

Signed-off-by: Alan Jowett <[email protected]>

---------

Signed-off-by: Alan Jowett <[email protected]>
Co-authored-by: Alan Jowett <[email protected]>
@shankarseal shankarseal mentioned this pull request Mar 22, 2024
shankarseal pushed a commit to shankarseal/ebpf-for-windows that referenced this pull request Mar 22, 2024
* Switch synchronous thread to thread_local storage

Signed-off-by: Alan Jowett <[email protected]>

* Add dedicated calls to cleanup device handles on thread attach/detach

Signed-off-by: Alan Jowett <[email protected]>

* Add dedicate test to stress device handling

Signed-off-by: Alan Jowett <[email protected]>

---------

Signed-off-by: Alan Jowett <[email protected]>
Co-authored-by: Alan Jowett <[email protected]>
@dthaler dthaler added the optimization Affects perf but not correctness or applicability label Mar 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
optimization Affects perf but not correctness or applicability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants