microsoft / DMF

Driver Module Framework
MIT License
311 stars 78 forks source link

Unexpected behaviour of ScheduledTask module #150

Closed nefarius closed 3 years ago

nefarius commented 3 years ago

Greetings!

First of all; this might be on me for misconfiguring or misusing this module so let me quickly summarize what I try to achieve. I have a device which requires a buffer sent to periodically while it's powered on, for example like every 10ms and/or sooner on demand.

Therefore I chose the ScheduledTask module which offers a deferred and "retry on success" pattern. This is unfortunately not working for me (the callback as seen below is only fired once after invoked), here's my setup:

DMF_MODULE_ATTRIBUTES moduleAttributes;
DMF_CONFIG_ScheduledTask dmfSchedulerCfg;

// ... other logic ...

DMF_CONFIG_ScheduledTask_AND_ATTRIBUTES_INIT(
&dmfSchedulerCfg,
&moduleAttributes
);

dmfSchedulerCfg.EvtScheduledTaskCallback = DMF_OutputReportScheduledTaskCallback;
dmfSchedulerCfg.CallbackContext = deviceContext;
dmfSchedulerCfg.PersistenceType = ScheduledTask_Persistence_NotPersistentAcrossReboots;
dmfSchedulerCfg.ExecutionMode = ScheduledTask_ExecutionMode_Deferred;
dmfSchedulerCfg.ExecuteWhen = ScheduledTask_ExecuteWhen_Other;
dmfSchedulerCfg.TimerPeriodMsOnSuccess = 10;
dmfSchedulerCfg.TimerPeriodMsOnFail = 10;

DMF_DmfModuleAdd(
DmfModuleInit,
&moduleAttributes,
WDF_NO_OBJECT_ATTRIBUTES,
&deviceContext->OutputReportScheduler
);

Here's my callback:

ScheduledTask_Result_Type
DMF_OutputReportScheduledTaskCallback(
    _In_ DMFMODULE DmfModule,
    _In_ VOID* CallbackContext,
    _In_ WDF_POWER_DEVICE_STATE PreviousState)
{
    // ... send buffer to device ...

    return ScheduledTask_WorkResult_SuccessButTryAgain;
}

I invoke it like so (which works, but only once until called again):

DMF_ScheduledTask_ExecuteNowDeferred(
        Context->OutputReportScheduler,
        Context
    );

Before I do a deep-dive into dissecting the module; any obvious error here in my thinking of what the module can do for me or the configuration I posted? Thanks!

samtertzakian commented 3 years ago

Sorry for delayed response.

See resolution below...

samtertzakian commented 3 years ago

Ok, I see what the issue is:

You need to use:

dmfSchedulerCfg.ExecuteWhen = ScheduledTaskExecuteWhenPrepareHardware; or dmfSchedulerCfg.ExecuteWhen = ScheduledTaskExecuteWhenD0Entry;

When you do so you do not use:

DMF_ScheduledTask_ExecuteNowDeferred() or DMF_ScheduledTask_ExecuteNow() Instead the scheduled task will be automatically started when PrepareHardware or D0Entry happen.

In this case, the scheduled task callback will be called automatically and repeatedly as you expected.

You used:

dmfSchedulerCfg.ExecuteWhen = ScheduledTaskExecuteWhenOther;

In this case you also used: DMF_ScheduledTask_ExecuteNowDeferred() or DMF_ScheduledTask_ExecuteNow()

But when you do so using the Method, the automatic retry does not happen (by design).

This is not intuitive and can be considered to be a bug. At the minimum, the documentation and comments in the code will be updated to reflect this behavior. But, if possible, we will see if we can update the code so that "Other" mode allows periodic call of Scheduled Task so that it works as one would expect.

Thank you for your feedback.

Please contact dmf-feedback@microsoft.com for faster responses.

nefarius commented 3 years ago

Thank you very much for the insights, I will test with ScheduledTask_ExecuteWhen_D0Entry then, should be alright.

Appreciate the addition to documentation!

One more question though; am I then by design allowed to still call DMF_ScheduledTask_ExecuteNowDeferred() "in between" when I need to not await the specified timeout value? I'd like to utilize it that way to send a packet to my device ASAP when necessary and sticking to just using the scheduler callback would allow me to not care about synchronization as the only sending code would remain in that callback.

Cheers!

samtertzakian commented 3 years ago

Yes, you instantiate using ScheduledTask_ExecuteWhen_D0Entry. Then, you can use DMF_ScheduledTask_ExecuteNow() to cause the same callback to be immediately called. In this case, however, you may need to acquire and release a lock in the callback to make sure that the deferred callback using timer does not race with your immediate execution. In fact, there is a comment in the code that explains that. If I recall, we specifically added DMF_ScheduledTask_ExecuteNow() for the exact purpose you describe.

Having said all that, I will update the documentation to explain all this (and possibly add a Method to do what you originally intended). I will keep this issue open until the documentation is updated. Let me know if you have any other questions. Thank you for the feedback.

nefarius commented 3 years ago

Yes, you instantiate using ScheduledTask_ExecuteWhen_D0Entry. Then, you can use DMF_ScheduledTask_ExecuteNow() to cause the same callback to be immediately called. In this case, however, you may need to acquire and release a lock in the callback to make sure that the deferred callback using timer does not race with your immediate execution. In fact, there is a comment in the code that explains that. If I recall, we specifically added DMF_ScheduledTask_ExecuteNow() for the exact purpose you describe.

It works now exactly as I need it with these adjustments, thanks! Also introduced a lock just to be safe.

samtertzakian commented 3 years ago

The pending PR has a fix for this issue: https://github.com/microsoft/DMF/pull/178

samtertzakian commented 3 years ago

Resolved by Release v1.1.98. Please use new DMF_ScheduledTask_ExecuteNowDeferredEx() Method. It honors the return value correctly.