Correct behavior of sched_lock() in SMP mode

[patacongo]

The function sched_lock() is used to disable preemption. If sched_lock() is called then the currently executing task will not be suspended until the scheduler is unlocked via sched_unlock().

The intent of sched_lock() is to simply keep a task in place. However, this function has been misused in many places (largely by me, I confess). In the single CPU case, sched_lock() has the side effect of giving the single CPU exclusive access access to everything. That is because if only that task can run on the CPU, then no other task can run and, hence, sched_lock() is an easy way to get a critical section without using enter/leave_critical_section().

That usage is incompatible with the current behavior of sched_lock() in the multiple CPU SMP case. In that case, the thread is locked in place on one CPU but other tasks may run on other CPUs: It does not create a critical section.

Logic was added in the current SMP to make the SMP behavior more like the single CPU behavior. That behavior is very complex. In summary: No new tasks are able to run on any CPU. No new higher priority tasks will be allowed to run on any CPU. If the running task on any CPU suspends itself, only the IDLE task will be permitted to run on the CPU.

Ideally, sched_lock() should only have the effect of keeping the currently running task in place on the local CPU. But currently sched_lock() does a lot more. The reason for that is because sched_lock() is also trying to enforce a critical section. I believe that that behavior is wrong and should not be a part of sched_lock().

I believe that using sched_lock() as a critical section is a misuse of sched-lock(). Its effect should be only that it keeps the currently running task on the CPU in place.

I propose that we carefully examine the use of sched_lock() throughout the OS. Proper uses of sched_lock() would be, for example, in timing operations to prevent the task from being suspended while it is performing a time calculation. Incorrect use of sched_lock() would be when it is used in order to create a critical section and, for example, get exclusive access to a resource. Those uses of sched_lock() should be replaced with calls to enter_critical_section().

If we do this, then the behavior of sched_lock() would be greatly simplified. That is, however, would be a pretty big job but, I think, necessary to do this right. That are a lot of instances of sched_lock() that would have to be examined.

Require to resolve Issue #1138

[yamt]

i have two comments

• to me, enter_critical_section() is another primitive with unclear semantics. are you interested in more fine grained locking? or, do you think the "global irq lock" approach is good enough for what nuttx is for?

• how about introducing a set of new api with clear semantics and name, say, preempt_disable().

[xiaoxiang781216]

i have two comments

• to me, enter_critical_section() is another primitive with unclear semantics. are you interested in more fine grained locking? or, do you think the "global irq lock" approach is good enough for what nuttx is for?

Here is the similar question: #1144

• how about introducing a set of new api with clear semantics and name, say, preempt_disable().

Does sched_lock equivalent to preempt_disable?

[yamt]

Does sched_lock equivalent to preempt_disable?

ideally it should be, if i read @patacongo's comment correctly.
it would allow us to replace sched_lock gradually.