Duplicate #625 : Fix xTaskNotifyWait & ulTaskNotifyTake determinism #719

kar-rahul-aws · 2023-07-24T18:55:50Z

Description

Fixes the bug described in this issue:
#612

One unorthodox thing being done here, which I feel should be pointed out, is that I am calling vTaskSuspendAll() from inside a critical section. This is done to minimize the performance impact of this change, since one of the main benefits of task notifications is that they are faster than the other primitives. The only worry I see here is that it may cause issues with certain ports if they do tricky things inside portSOFTWARE_BARRIER() or portMEMORY_BARRIER(), but I cannot think of any examples.

Test Steps

The code seems to work on an RP2040.

Checklist:

I have tested my changes. No regression in existing tests.
I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

#625

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

karver8 · 2023-07-31T23:05:15Z

I am curious why the PR I submitted was copied? I am guessing it is just easier this way, which is fine, though I would appreciate if I was made author of the first commit. Some sort of record of my contribution on GitHub would be nice. Obviously you don't have to do this.

Anyways this looks like what I figured would need to happen to get those tests to pass, I have been waiting for a response to my questions about these tests but never received one. Thanks for doing this @kar-rahul-aws.

kar-rahul-aws · 2023-08-01T03:21:53Z

Hi @karver8
Yes you are right, this duplicate PR was opened , for easier merge conflict resolution since the source code for FreeRTOS Kernel underwent a lot of changes since your PR. I have kept this PR as a draft , and also your old PR open, since we are still working on which one to merge , once the unit tests are fixed. Rest assured, your name will added as the first author for this very commit . Thanks again for the contribution.
For the unit test fail issue, I have opened a PR . You can have a look and see if this is what you had intended to do .

karver8 · 2023-09-02T21:27:25Z

@kar-rahul-aws Thanks for doing this. The addition of listLIST_IS_EMPTY_Expect() is what I wrote down in my notes a while back so that PR seems good to me.