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

mcs: more uniformly handle ready and release queue updates #1176

Merged
merged 1 commit into from
May 23, 2024

Conversation

michaelmcinerney
Copy link
Contributor

This introduces library functions for updating the linked lists which use the tcbSchedNext and tcbSchedPrev pointers of a TCB, and uses these to perform the updates to the ready queues and the release queue.

In order to accommodate this, ksReleaseQueue is now of type tcb_queue_t.

@michaelmcinerney michaelmcinerney added the MCS issues about the mixed-criticality system config label Jan 30, 2024
@michaelmcinerney michaelmcinerney self-assigned this Jan 30, 2024
@lsf37 lsf37 added the verification Needs formal verification input/change, or is motivated by verification label Jan 30, 2024
@lsf37
Copy link
Member

lsf37 commented Jan 30, 2024

This also affects non-MCS configs, so we can only merge once we have ported the verification of it to the master branch as well.

Maybe some words on the background of this: factoring out the queue updates means we only have to verify them once, not twice. The change in type is there to achieve that genericity, and the head/tail type is the same mechanism used for tcb_queue_t, so it's just re-using those ideas. The operations are slightly different, so there was no point trying to make it even more generic (and would probably be fighting C too much).

Copy link
Contributor

@Indanz Indanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks functionally correct to me.

include/object/tcb.h Show resolved Hide resolved
include/object/tcb.h Show resolved Hide resolved
src/kernel/boot.c Show resolved Hide resolved
src/object/tcb.c Show resolved Hide resolved
src/object/tcb.c Outdated Show resolved Hide resolved
src/object/tcb.c Show resolved Hide resolved
src/object/tcb.c Show resolved Hide resolved
src/object/tcb.c Outdated Show resolved Hide resolved
src/object/tcb.c Outdated Show resolved Hide resolved
@Indanz
Copy link
Contributor

Indanz commented Jan 30, 2024

The operations are slightly different, so there was no point trying to make it even more generic

It's mostly the insert operation that's different, but that can be solved by first finding the spot and then inserting the item. The rest seems pretty generic. Still at this point it's probably not quite worth it. Especially tcbEPDequeue seems to be eye wateringly optimistic about being correctly used, but it does shave off a few cycles here and there.

(and would probably be fighting C too much).

The problem is not C, in C you would let the next/prev pointers point to queue_t in each item and have a macro like container_of which uses offsetof() to get a pointer to the parent object from the next/prev pointers. However, I doubt that kind of pointer arithmetic and type casting is legal in verified seL4 C code.

It's worth doing if more complex structures than lists are used in the future, like heaps or trees.

@lsf37 lsf37 added the hw-test sel4test hardware builds + runs for this PR label Feb 6, 2024
@lsf37 lsf37 force-pushed the michaelm/linked_list branch 2 times, most recently from de51820 to c9f7a50 Compare April 15, 2024 03:09
This introduces library functions for updating the linked lists which
use the tcbSchedNext and tcbSchedPrev pointers of a TCB, and uses these
to perform the updates to the ready queues and the release queue.

In order to accommodate this, ksReleaseQueue is now of type tcb_queue_t.

Co-authored-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
Signed-off-by: Michael McInerney <michael.mcinerney@proofcraft.systems>
Copy link
Member

@lsf37 lsf37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now ready to merge if all tests pass.

@lsf37 lsf37 merged commit 686bba4 into master May 23, 2024
76 of 83 checks passed
@lsf37 lsf37 deleted the michaelm/linked_list branch May 23, 2024 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hw-test sel4test hardware builds + runs for this PR MCS issues about the mixed-criticality system config verification Needs formal verification input/change, or is motivated by verification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants