> On Jul 20, 2015, at 12:00 PM, Sorin Vinturis
> <[email protected]> wrote:
>
> In order to support IRP cancelling mechanism for pending IRPs, all
> tunnel filter requests, VXLAN create/delete tunnel, need to be
> processed iteratively.
>
> Signed-off-by: Sorin Vinturis <[email protected]>
hi Sorin,
I had a question regarding latency of processing the requests. Looks good
otherwise. A v4 should be good to go.
> - do
> - {
> + do {
> if (!InterlockedCompareExchange(
> (LONG volatile *)&threadCtx->listRequests.numEntries, 0, 0)) {
> OVS_LOG_INFO("Nothing to do... request list is empty.");
> @@ -1076,38 +1056,25 @@
> OvsTunnelFilterRequestListProcess(POVS_TUNFLT_THREAD_CONTEXT threadCtx)
> }
> inTransaction = TRUE;
>
> - InitializeListHead(&head);
> - OvsTunnelFilterRequestPopList(&threadCtx->listRequests, &head,
> &count);
> -
> - LIST_FORALL_SAFE(&head, link, next) {
> - request = CONTAINING_RECORD(link, OVS_TUNFLT_REQUEST, entry);
> + while (NULL != (request = OvsTunnelFilterRequestPop(
> + &threadCtx->listRequests))) {
minor: indentation can be improved to:
while (NULL !=
(request = OvsTunnelFilterRequestPop(&threadCtx->listRequests))) {
>
> status = OvsTunnelFilterExecuteAction(threadCtx->engineSession,
> request);
> +
> + /* Complete the IRP with the last operation status. */
> + OvsTunnelFilterCompleteRequest(request, status);
> +
> + OvsFreeMemory(request);
> + request = NULL;
> +
> if (!NT_SUCCESS(status)) {
> - RemoveEntryList(&request->entry);
> - count--;
> -
> - /* Complete the IRP with the failure status. */
> - OvsTunnelFilterCompleteRequest(request->irp,
> - request->callback,
> - request->context,
> - status);
> - OvsFreeMemory(request);
> - request = NULL;
> - } else {
> - error = FALSE;
> + break;
> }
> }
>
What happens if there are 10 requests to process, and the 5th one fails. You’d
bail out of processing commit the 4 and go back into calling
KeWaitForMultipleObjects() in OvsTunnelFilterThreadProc(). What is the trigger
to processing the remaining 5 request, esp. if they are legitimate requests and
there’s a good chance that they’ll succeed.
Also, if you are worried about the number of requests being committed per
transaction, maybe you should submit them in batches.
Does KeSetEvent() queue up wakeup events?
-- Nithin
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
