Yeah, but this macro is working because of coincidental match with
variable prev already defined in the scope.
On 5/6/25 23:19, Alan C. Assis wrote:
This file:
https://github.com/apache/nuttx/commit/9de9f8168d6de8eab8d3a97aac21aacc4e84dd84#diff-ac1d3cade3d9ab380d40fe31f8c006b3035c7b53d04c28e23d9f1ce9a176572c
On Tue, May 6, 2025 at 1:18 PM Alan C. Assis <acas...@gmail.com> wrote:
Hi Serg,
I did an analysis here and this macro was introduced in Jan 10 2024, so it
was more than 1 year ago:
https://github.com/apache/nuttx/commit/9de9f8168d6de8eab8d3a97aac21aacc4e84dd84
BR,
Alan
On Tue, May 6, 2025 at 1:12 PM Alan C. Assis <acas...@gmail.com> wrote:
WOW! Nice catch!
Question to some people with more experience in the scheduler:
Why wasn't this issue detected before? It was added more than 9 months
ago.
Is there some way to test and enforce that nxsched_process_delivered()
and other schedule functions are working as expected?
BR,
Alan
On Tue, May 6, 2025 at 11:14 AM Serg Podtynnyi <s...@podtynnyi.com.invalid>
wrote:
Hi, I am still researching the sched problem on my config
Found this macro code from last September, looks like pre vs prev
typo, right?
#definedq_insert_mid(pre,mid,next)\
do\
{\
mid->flink =next;\
mid->blink =prev;\
pre->flink =mid;\
next->blink =mid;\
}\
while(0)
On 4/24/25 15:06, hujun260 wrote:
The lockcount can be understood as a local variable, so no race
conditions will occur.
编辑
分享
At 2025-04-24 15:54:48, "Serg Podtynnyi"<s...@podtynnyi.com.INVALID>
wrote:
Hi, I tried my best to understand the logic behind unlock and
merge_pending routines and come up with this fix that works for me.
I split the lockcount handling in 2 parts when lockcount is > 1 just
decrement fast as always,
but when it's 1 we enter critical section and make it 0 inside
it(pre-emption is enabled).
I think there is a small time after decrement in the if statement and
enter_critical_section_wo_note() in original code where can race
happen.
Does this make sense? Sorry if it's a dumb question - scheduler stuff
with SMP is rocket-science level for me yet.
diff --git a/sched/sched/sched_unlock.c b/sched/sched/sched_unlock.c
index 4c7d681454..15fc39b546 100644
--- a/sched/sched/sched_unlock.c
+++ b/sched/sched/sched_unlock.c
@@ -65,13 +65,14 @@ void sched_unlock(void)
DEBUGASSERT(rtcb == NULL || rtcb->lockcount > 0);
- /* Check if the lock counter has decremented to zero. If so,
- * then pre-emption has been re-enabled.
- */
-
- if (rtcb != NULL && --rtcb->lockcount == 0)
+ if (rtcb != NULL && rtcb->lockcount > 1)
+ {
+ rtcb->lockcount--;
+ }
+ else if (rtcb != NULL && rtcb->lockcount == 1)
{
irqstate_t flags = enter_critical_section_wo_note();
+ rtcb->lockcount = 0;
/* Note that we no longer have pre-emption disabled. */
--
2.49.0
On 4/22/25 14:32, hujun260 wrote:
It seems to have nothing to do with the race condition, and since the
rtcb is this_task, it will only be modified by one CPU.<br/><br/>As for why
your modification can solve the problem, I haven't figured it out yet. Can
you further analyze the reasons for the failure to boot?
At 2025-04-22 13:25:13, "Serg Podtynnyi"<s...@podtynnyi.com.INVALID>
wrote:
Hi All, Gregory, Sebastien,
I'm porting NuttX on rp2350(pico2 board inside ClockWork PicoCalc)
and
having trouble getting SMP (2-core) config running
without patching the sched lock/unlock routines.
I am looking at the commit
https://github.com/apache/nuttx/commit/914ae532e68bf4ca75c758d6f541a1d2fcdce8c3
and reverting it helps to boot the system.
This the the patch to make it work for me, I updated the current
lock/unlock by moving the enter_critical_section above the access to
rtcb->lockcount
Do you think it could be the case of a race condition with
rtcb->lockcount ?
PS
It's almost 11 years since I last touched NuttX - it's a please to
work
with it again.
From 86fa0a65fdd37804154faf3db52e2826281cdfbd Mon Sep 17 00:00:00
2001
From: Serg Podtynnyi<s...@podtynnyi.com>
Date: Tue, 22 Apr 2025 11:37:48 +0700
Subject: sched: update lock/unlock critical section entry
Level up enter_critical_section in sched lock/unlock to cover
rtcb->lockcount access
Signed-off-by: Serg Podtynnyi<s...@podtynnyi.com>
---
sched/sched/sched_lock.c | 4 ++--
sched/sched/sched_unlock.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sched/sched/sched_lock.c b/sched/sched/sched_lock.c
index d8f75e7c90..9cc8da3df4 100644
--- a/sched/sched/sched_lock.c
+++ b/sched/sched/sched_lock.c
@@ -82,18 +82,18 @@ void sched_lock(void)
* operations on this thread (on any CPU)
*/
+ irqstate_t flags = enter_critical_section_wo_note();
if (rtcb != NULL && rtcb->lockcount++ == 0)
{
#if (CONFIG_SCHED_CRITMONITOR_MAXTIME_PREEMPTION >= 0) || \
defined(CONFIG_SCHED_INSTRUMENTATION_PREEMPTION)
- irqstate_t flags = enter_critical_section_wo_note();
/* Note that we have pre-emption locked */
nxsched_critmon_preemption(rtcb, true,
return_address(0));
sched_note_preemption(rtcb, true);
- leave_critical_section_wo_note(flags);
#endif
}
+ leave_critical_section_wo_note(flags);
}
}
diff --git a/sched/sched/sched_unlock.c b/sched/sched/sched_unlock.c
index 4c7d681454..d21e007587 100644
--- a/sched/sched/sched_unlock.c
+++ b/sched/sched/sched_unlock.c
@@ -69,9 +69,9 @@ void sched_unlock(void)
* then pre-emption has been re-enabled.
*/
+ irqstate_t flags = enter_critical_section_wo_note();
if (rtcb != NULL && --rtcb->lockcount == 0)
{
- irqstate_t flags = enter_critical_section_wo_note();
/* Note that we no longer have pre-emption disabled. */
@@ -162,7 +162,7 @@ void sched_unlock(void)
}
#endif
- leave_critical_section_wo_note(flags);
}
+ leave_critical_section_wo_note(flags);
}
}
--
2.49.0
--
Serg Podtynnyi
-- Serg Podtynnyi
--
Serg Podtynnyi
--
Serg Podtynnyi
