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

Always use an overlapped structure when calling DeviceIoControl #30

Merged
merged 1 commit into from
Oct 19, 2020

Conversation

petrutlucian94
Copy link
Member

This change fixes deadlocks occurring when communicating with the
WNBD driver using DeviceIoControl[1][2].

We're setting FILE_FLAG_OVERLAPPED when opening the WNBD device
in order to be able to submit multiple requests simultaneously.
The Windows docs state that in this case, an OVERLAPED structure
must be passed to functions such as ReadFile or WriteFile without
explicitly mentioning DeviceIoControl.

Apparently DeviceIoControl can hang if we don't pass an overlapped
structure, especially when having a high number of concurrent
requests. This is likely related to the event wait involved.

We're going to update the libwnbd Ioctl functions, adding an
optional LPOVERLAPPED argument. When not provided by the caller,
we're initalizing an internal OVERLAPED structure and wait for
the operation on the behalf of the caller, mimicking the Windows
behavior.

We're trying to re-use overlapped structures and events as much
as possible, especially in the IO path. The IO dispatch loop
will reuse the same events but we can't do much about
"WnbdSendResponse" since that will be called by libwnbd consumers
from arbitrary threads. We're adding "WnbdSendResponseEx" though,
letting the caller provide an overlapped structure.

Another advantage of adding overlapped parameters is that we allow
libwnbd consumers to perform any Ioctl call asynchronously.

[1] profiler counters: http://paste.openstack.org/raw/797772/
[2] wnbd / ceph counters: http://paste.openstack.org/raw/799072/

Signed-off-by: Lucian Petrut lpetrut@cloudbasesolutions.com

This change fixes deadlocks occurring when communicating with the
WNBD driver using DeviceIoControl[1][2].

We're setting FILE_FLAG_OVERLAPPED when opening the WNBD device
in order to be able to submit multiple requests simultaneously.
The Windows docs state that in this case, an OVERLAPED structure
must be passed to functions such as ReadFile or WriteFile without
explicitly mentioning DeviceIoControl.

Apparently DeviceIoControl can hang if we don't pass an overlapped
structure, especially when having a high number of concurrent
requests. This is likely related to the event wait involved.

We're going to update the libwnbd *Ioctl* functions, adding an
optional LPOVERLAPPED argument. When not provided by the caller,
we're initalizing an internal OVERLAPED structure and wait for
the operation on the behalf of the caller, mimicking the Windows
behavior.

We're trying to re-use overlapped structures and events as much
as possible, especially in the IO path. The IO dispatch loop
will reuse the same events but we can't do much about
"WnbdSendResponse" since that will be called by libwnbd consumers
from arbitrary threads. We're adding "WnbdSendResponseEx" though,
letting the caller provide an overlapped structure.

Another advantage of adding overlapped parameters is that we allow
libwnbd consumers to perform any *Ioctl* call asynchronously.

[1] profiler counters: http://paste.openstack.org/raw/797772/
[2] wnbd / ceph counters: http://paste.openstack.org/raw/799072/

Signed-off-by: Lucian Petrut <lpetrut@cloudbasesolutions.com>
@aserdean aserdean self-requested a review October 19, 2020 09:19
@aserdean
Copy link
Contributor

Acked-by: Alin Gabriel Serdean aserdean@cloudbasesolutions.com

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.

2 participants