You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by Serg Podtynnyi <se...@podtynnyi.com.INVALID> on 2025/05/06 14:13:55 UTC
Re: sched lock/unlock issue on rp2350
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"<se...@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"<se...@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<se...@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<se...@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
Re: sched lock/unlock issue on rp2350
Posted by "Alan C. Assis" <ac...@gmail.com>.
True, so this typo didn't create a side effect, right?
Are you git bisecting to find the root cause of the issue?
BR,
Alan
On Wed, May 7, 2025 at 12:08 AM Serg Podtynnyi <se...@podtynnyi.com.invalid>
wrote:
> 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 <ac...@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 <ac...@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
> <se...@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"<se...@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"<serg@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<se...@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<se...@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
>
>
Re: sched lock/unlock issue on rp2350
Posted by Serg Podtynnyi <se...@podtynnyi.com.INVALID>.
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 <ac...@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 <ac...@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 <se...@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"<se...@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"<se...@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<se...@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<se...@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
Re: sched lock/unlock issue on rp2350
Posted by "Alan C. Assis" <ac...@gmail.com>.
This file:
https://github.com/apache/nuttx/commit/9de9f8168d6de8eab8d3a97aac21aacc4e84dd84#diff-ac1d3cade3d9ab380d40fe31f8c006b3035c7b53d04c28e23d9f1ce9a176572c
On Tue, May 6, 2025 at 1:18 PM Alan C. Assis <ac...@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 <ac...@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 <se...@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"<se...@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"<se...@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<se...@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<se...@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
>>>
>>>
Re: sched lock/unlock issue on rp2350
Posted by "Alan C. Assis" <ac...@gmail.com>.
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 <ac...@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 <se...@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"<se...@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"<se...@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<se...@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<se...@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
>>
>>
Re: sched lock/unlock issue on rp2350
Posted by "Alan C. Assis" <ac...@gmail.com>.
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 <se...@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"<se...@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"<se...@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<se...@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<se...@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
>
>