jlaitine commented on code in PR #16673:
URL: https://github.com/apache/nuttx/pull/16673#discussion_r2228006231


##########
sched/sched/sched_unlock.c:
##########
@@ -69,10 +69,12 @@ void sched_unlock(void)
        * then pre-emption has been re-enabled.
        */
 
-      if (rtcb != NULL && --rtcb->lockcount == 0)

Review Comment:
   Here is a quick analysis of issues that the current code here causes:
   1) This code would be in wrong place, causing something else accounted into 
being inside sched_lock:
   
   ```
   /* Note that we no longer have pre-emption disabled. */
   
   nxsched_critmon_preemption(rtcb, false, return_address(0));
   sched_note_preemption(rtcb, false);
   
   ```
   
   Later on there is the logic to check if there has been tasks pending because 
of the sched lock. Before this PR, there is a call to nxsched_merge_pending 
(and for single-cpu case there still is). This call is delayed if there is a 
schedule in between, leaving (possibly) higher priority task in pending list 
and scheduling something else in. So it is delaying the pending task schedule.
   
   Something I still want to point out about the linux kernel comment; Linux 
scheduling supports many different scheduling schemes, and it is heavily 
optimized for CFQ (completely fair queuing). Many constructs are unnecessary 
and complicated if you just want to do priority based scheduling; this is a 
real-time os.
   
   In addition, copying stuff from linux kernel is dangerous from the license 
point of view - linux is GPLv2. Knowing how the linux kernel works and using 
this knowledge to make own code is of course fine. But this is a fine line! 
Technically it is not even ok to study the linux kernel for the purpose of 
implementing the same in NuttX.
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to