sched: fix deadline init value and add a test case

[TaiJuWu]

The init value of deadline scheduler algorithm should be INT_MAX.
If this value set to 0, it means normal task is going to run before deadline task.

[TaiJuWu]

Hi @andyross @peter-mitsis @cfriedt, Could you take a look?
I think this is a critical bug about deadline scheduler if I'm right.

[cfriedt]

@TaiJuWu - the provided test definitely needs to be elaborated a bit more and it looks like there are still some races.

I haven't looked to see if we already have sufficient coverage for the deadline scheduler but it might be worth looking at how some of the other scheduler experiments have been set up.

Note: the argument to k_thread_deadline_set() has the same units as the value returned from k_cycle_get_32(), so that can be used to form the experiment in terms of time durations, which is maybe easier to deal with in terms of setting up the right conditions.

From what I understand, the lower the deadline, the earlier a thread must be scheduled.

If we're lacking a kind of stress test for the deadline scheduler it might be useful to hash that out before revising.

[TaiJuWu]

@TaiJuWu - the provided test definitely needs to be elaborated a bit more and it looks like there are still some races.

The test environment is 1 cpu so it's not race.
You can see here.

I haven't looked to see if we already have sufficient coverage for the deadline scheduler but it might be worth looking at how some of the other scheduler experiments have been set up.

Note: the argument to k_thread_deadline_set() has the same units as the value returned from k_cycle_get_32(), so that can be used to form the experiment in terms of time durations, which is maybe easier to deal with in terms of setting up the right conditions.

From what I understand, the lower the deadline, the earlier a thread must be scheduled.

You are right, I have same idea but the init valuse is 0.
here is the place which user set deadline and always larger than 0.
The code compare priority is here so you can find the normal task will run first.

If we're lacking a kind of stress test for the deadline scheduler it might be useful to hash that out before revising.

[peter-mitsis]

@TaiJuWu - After an initial reading, I think that you are on to something here. However, my poor brain is going to need some more time to parse through this.

[TaiJuWu]

@TaiJuWu - After an initial reading, I think that you are on to something here. However, my poor brain is going to need some more time to parse through this.

Thanks for your response.
If I’m wrong, please tell me.
Maybe this patch is a bad solution or I missed something.

[peter-mitsis]

After digging into it more, I think the current code is correct for the model that it uses. There are two different time scales at play here which I will call T(x) and t(x). T(x) represents the lower 32-bits of absolute time (k_cycle_get_32()). t(x) represents the sliding window of relative time such that t(0) = T(now), or t(0) = T(k_cycle_get_32()). When we call k_thread_deadline_set(), the deadline is specified in relative time. Internally we convert it to T(x) absolute time using deadline + now.

Two other important things to note ...

1. Unlike T(x) which is 32 bits unsigned, t(x) is 32 bits signed.
2. The number of cycles between the “first” deadline in the scheduler queue and the “last” deadline must be less than 2^31

Combining this altogether, we see that when k_cycle_get_32() exceeds 0x80000000, this means that that T(0) effectively becomes a future point in time, and z_sched_prio_cmp() will treat it as such.

[TaiJuWu]

After digging into it more, I think the current code is correct for the model that it uses. There are two different time scales at play here which I will call T(x) and t(x). T(x) represents the lower 32-bits of absolute time (k_cycle_get_32()). t(x) represents the sliding window of relative time such that t(0) = T(now), or t(0) = T(k_cycle_get_32()). When we call k_thread_deadline_set(), the deadline is specified in relative time. Internally we convert it to T(x) absolute time using deadline + now.

Two other important things to note ...

1. Unlike T(x) which is 32 bits unsigned, t(x) is 32 bits signed.
2. The number of cycles between the “first” deadline in the scheduler queue and the “last” deadline must be less than 2^31

Combining this altogether, we see that when k_cycle_get_32() exceeds 0x80000000, this means that that T(0) effectively becomes a future point in time, and z_sched_prio_cmp() will treat it as such.

Thanks for your answers. I need more time to think this issue more deep.

And I want to check a thing which is normal task should run after deadline task, right?
In my new testcase, the normal task will run first and deadline task start to run. It seems not our expection.

[andyross]

This isn't the way deadline comparisons work. The units of the deadline field are in units of cycles (c.f. k_cycle_get_32()) with expected rollover. They aren't absolute. Picking a different number just means that the scheduling vs. threads that don't set their deadlines will be arbitrarily different.

Really the kernel doesn't support mixing deadline and non-deadline threads in the same priority. If you have EDF enabled, you're expected to set deadlines for threads that want to share a priority, or else to leave them unset (and thus equal, which then falls through to "traditional" round robin ordering).

FWIW, one thing that's been suggested in the past is having a way to "clear" a deadline (i.e. to recover equality/round-robin scheduling behavior), which the kernel doesn't provide. IIRC I had suggested at one point doing this when the priority changes, but you could add a separate call instead. If you're worried about this area of the code, that might be more productive.

[TaiJuWu]

This isn't the way deadline comparisons work. The units of the deadline field are in units of cycles (c.f. k_cycle_get_32()) with expected rollover. They aren't absolute. Picking a different number just means that the scheduling vs. threads that don't set their deadlines will be arbitrarily different.

Really the kernel doesn't support mixing deadline and non-deadline threads in the same priority. If you have EDF enabled, you're expected to set deadlines for threads that want to share a priority, or else to leave them unset (and thus equal, which then falls through to "traditional" round robin ordering).

FWIW, one thing that's been suggested in the past is having a way to "clear" a deadline (i.e. to recover equality/round-robin scheduling behavior), which the kernel doesn't provide. IIRC I had suggested at one point doing this when the priority changes, but you could add a separate call instead. If you're worried about this area of the code, that might be more productive.

I want to summarize your response.
In zephyr CONFIG_DEADLINE enabled, all threads should set deadline?
If not set, the thread will run first, right?
So all behavior is working as your expect?

IMHO, if users does not set deadline, a possible is they hope this task start running until deadline task finished.

[TaiJuWu]

After digging into it more, I think the current code is correct for the model that it uses. There are two different time scales at play here which I will call T(x) and t(x). T(x) represents the lower 32-bits of absolute time (k_cycle_get_32()). t(x) represents the sliding window of relative time such that t(0) = T(now), or t(0) = T(k_cycle_get_32()). When we call k_thread_deadline_set(), the deadline is specified in relative time. Internally we convert it to T(x) absolute time using deadline + now.

Two other important things to note ...

1. Unlike T(x) which is 32 bits unsigned, t(x) is 32 bits signed.
2. The number of cycles between the “first” deadline in the scheduler queue and the “last” deadline must be less than 2^31

Combining this altogether, we see that when k_cycle_get_32() exceeds 0x80000000, this means that that T(0) effectively becomes a future point in time, and z_sched_prio_cmp() will treat it as such.

I notice a truth which is if k_cycle_get_32() exceeds 0x80000000 and the deadline is smaller than 0 (This is allowed because we don't check the parameter of deadline).
We can't distinguish the job should be done in future or in past.

[github-actions]

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.