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

Library - Pull a batch of Kernel request and process them by a thread pool #981

Closed
Liryna opened this issue Mar 30, 2021 · 4 comments
Closed

Comments

@Liryna
Copy link
Member

Liryna commented Mar 30, 2021

The kernel has a new option that can batch requests instead of sending them one by one. Enabling this option with the current library would not make sense as one thread might be loaded with requests and the others will be waiting.
Therefore we should move the library to a single thread that pulls a batch of Kernel requests and processes them by a thread pool.

dokany/sys/dokan.h

Lines 289 to 292 in 9d7bc12

// Allow I/O requests to be conveyed to user mode in batches, rather than
// strictly one for each DeviceIoControl that the DLL issues to fetch a
// request.
BOOLEAN AllowIpcBatching;

This is a great performance improvement.

This feature is waiting for external contributions to be happening. I would be glad to review any PR!

@Liryna
Copy link
Member Author

Liryna commented Mar 30, 2021

It might help to take a look at @Corillian PR that already used a thread pool except each of them were pulling the kernel request at the same time.
https://github.com/dokan-dev/dokany/blob/Corillian-asyncio/dokan/dokan.c#L1445-L1475

@Liryna
Copy link
Member Author

Liryna commented May 15, 2021

Adding more details for anyone looking into this. Please do not hesitate if you have any questions!

Batched events are built by NotificationLoop in the Kernel.

VOID NotificationLoop(__in PIRP_LIST PendingIoctls, __in PIRP_LIST WorkQueue,

The EVENT_CONTEXT from the WorkQueue -> workItem->EventContext list are all append to the PendingIoctls buffers that directly comes from the userland function DokanLoop.

dokany/sys/notification.c

Lines 298 to 319 in 4208150

workItemListEntry = RemoveHeadList(&WorkQueue->ListHead);
workItem = CONTAINING_RECORD(workItemListEntry, DRIVER_EVENT_CONTEXT,
ListEntry);
workItemBytes = workItem->EventContext.Length;
// Buffer is not specified or short of length (this may mean we filled the
// space in one of the DLL's buffers in batch mode). Put the IRP back in
// the work queue; it will have to go in a different buffer.
if (currentIoctlBuffer == NULL
|| currentIoctlBufferBytesRemaining < workItemBytes) {
InsertTailList(&WorkQueue->ListHead, &workItem->ListEntry);
currentIoctl = NULL;
continue;
}
// Send the work item back in the response to the current IOCTL.
RtlCopyMemory(currentIoctlBuffer, &workItem->EventContext, workItemBytes);
currentIoctlBufferBytesRemaining -= workItemBytes;
currentIoctlBuffer += workItemBytes;
currentIoctlIrpEntry->SerialNumber += workItemBytes;
if (workItem->Completed) {
KeSetEvent(workItem->Completed, IO_NO_INCREMENT, FALSE);
}
ExFreePool(workItem);

The idea would be to have DokanLoop to be only used by one thread to pull the batched events from the driver. (Also grow the current buffer by 4 to batch more events).

dokany/dokan/dokan.c

Lines 448 to 458 in 31f8381

status = DeviceIoControl(
device, // Handle to device
FSCTL_EVENT_WAIT, // IO Control code
NULL, // Input Buffer to driver.
0, // Length of input buffer in bytes.
buffer, // Output Buffer from driver.
sizeof(char) *
EVENT_CONTEXT_MAX_SIZE, // Length of output buffer in bytes.
&returnedLength, // Bytes placed in buffer.
NULL // synchronous call
);

Unstack all EVENT_CONTEXT and forward them to a new thread pool that will process them.

dokany/dokan/dokan.c

Lines 485 to 528 in 31f8381

switch (context->MajorFunction) {
case IRP_MJ_CREATE:
DispatchCreate(device, context, DokanInstance);
break;
case IRP_MJ_CLEANUP:
DispatchCleanup(device, context, DokanInstance);
break;
case IRP_MJ_CLOSE:
DispatchClose(device, context, DokanInstance);
break;
case IRP_MJ_DIRECTORY_CONTROL:
DispatchDirectoryInformation(device, context, DokanInstance);
break;
case IRP_MJ_READ:
DispatchRead(device, context, DokanInstance);
break;
case IRP_MJ_WRITE:
DispatchWrite(device, context, DokanInstance);
break;
case IRP_MJ_QUERY_INFORMATION:
DispatchQueryInformation(device, context, DokanInstance);
break;
case IRP_MJ_QUERY_VOLUME_INFORMATION:
DispatchQueryVolumeInformation(device, context, DokanInstance);
break;
case IRP_MJ_LOCK_CONTROL:
DispatchLock(device, context, DokanInstance);
break;
case IRP_MJ_SET_INFORMATION:
DispatchSetInformation(device, context, DokanInstance);
break;
case IRP_MJ_FLUSH_BUFFERS:
DispatchFlush(device, context, DokanInstance);
break;
case IRP_MJ_QUERY_SECURITY:
DispatchQuerySecurity(device, context, DokanInstance);
break;
case IRP_MJ_SET_SECURITY:
DispatchSetSecurity(device, context, DokanInstance);
break;
case DOKAN_IRP_LOG_MESSAGE:
DispatchDriverLogs(device, context, DokanInstance);
default:
break;

To know if the buffer contains more events, we simply need to check if the remaining space can contain a EVENT_CONTEXT and if it is valid (EventContext.Length).

For information, IpcBatching also allows the library to send batches of responses to the Kernel.
It might just not be that great to implement it yet as the responses will be processed in the thread sending them instead of one thread for each response like now.
Also the code is for now disabled until a 2.x.x version. See

dokany/sys/event.c

Lines 524 to 529 in 4208150

// We break until 2.x.x - See function head comment
__pragma(warning(push))
__pragma(warning(disable : 4127))
if (1 == 1)
break;
__pragma(warning(pop))

@Liryna
Copy link
Member Author

Liryna commented May 25, 2021

The last version now has the option in the kernel if anyone would like to take a look at it https://github.com/dokan-dev/dokany/releases/tag/v1.5.0.1000

@Liryna
Copy link
Member Author

Liryna commented Jan 9, 2022

This is done as part of 2.0.0

@Liryna Liryna closed this as completed Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant