You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/09/03 07:46:29 UTC
[GitHub] [incubator-nuttx] michallenc opened a new pull request #4466: sched/wdog/wd_start.c: move wd_expiration() function back to while loop
michallenc opened a new pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466
## Summary
Recent change presented in commit 9116ed9247 (PR #4413) moved wd_expiration() outside
while loop. This was causing hard faults when running configuration with
enabled Tickless Mode with Alarm. The current change fixes this problem.
## Testing
Tested on Teensy 4.1 board. Probably worthy to do some other tests on different platforms with Alarm turn on if anyone has them available.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] michallenc edited a comment on pull request #4466: sched/wdog/wd_start.c: move wd_expiration() function back to while loop
Posted by GitBox <gi...@apache.org>.
michallenc edited a comment on pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466#issuecomment-913021761
I will do the debugging on imxrt1060-evk just to be sure the problem is really in the line 103 in wd_expiration() function as @xiaoxiang781216 sent. Probably will get into this after the weekend.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4466: sched/wdog/wd_start.c: prevent accessing watch-dog lag if head is NULL
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466#issuecomment-913471081
LGTM, @davids5 please take a look.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4466: sched/wdog/wd_start.c: move wd_expiration() function back to while loop
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466#issuecomment-912894804
> > Can you explain why move the wd_expiration to while loop can fix the problem.
>
> @GUIDINGLI My uderstanding is that when wd_expiration() is outside the loop that it might be called even if wdog == NULL which might cause the hard fault. That´s the only difference I can see there. The tests seems ok but I am also not much confident about this as I haven´t worked with watchdogs before.
From this line: https://github.com/apache/incubator-nuttx/blob/master/sched/wdog/wd_start.c#L101-L110
```
/* Check if the watchdog at the head of the list is ready to run */
if (((FAR struct wdog_s *)g_wdactivelist.head)->lag <= 0)
{
/* Process the watchdog at the head of the list as well as any
* other watchdogs that became ready to run at this time
*/
while (g_wdactivelist.head &&
((FAR struct wdog_s *)g_wdactivelist.head)->lag <= 0)
```
Look like the if statement may access lag when head is NULL. Since while statement check the same condition, and verify head isn't NULL before the check, the simple fix is remove if statement.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] michallenc commented on pull request #4466: sched/wdog/wd_start.c: move wd_expiration() function back to while loop
Posted by GitBox <gi...@apache.org>.
michallenc commented on pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466#issuecomment-912452167
> Can you explain why move the wd_expiration to while loop can fix the problem.
@GUIDINGLI My uderstanding is that when wd_expiration() is outside the loop that it might be called even if wdog == NULL which might cause the hard fault. That´s the only difference I can see there. The tests seems ok but I am also not much confident about this as I haven´t worked with watchdogs before.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] michallenc commented on pull request #4466: sched/wdog/wd_start.c: move wd_expiration() function back to while loop
Posted by GitBox <gi...@apache.org>.
michallenc commented on pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466#issuecomment-913021761
I will do the debugging on imxrt1060-evk just to be sure the problem is really in the line 103 in wd_expiration() function as @xiaoxiang781216 sent. Probably will get to this after the weekend.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] michallenc commented on pull request #4466: sched/wdog/wd_start.c: prevent accessing watch-dog lag if head is NULL
Posted by GitBox <gi...@apache.org>.
michallenc commented on pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466#issuecomment-913442795
Hi. The debugging proved that NuttX really crashes because that if statement. I returned wd_expiration() call back as presented in commit 9116ed9247 and removed the problematic if statement. @GUIDINGLI @xiaoxiang781216 @davids5 Could you take a look at this? Thanks.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 merged pull request #4466: sched/wdog/wd_start.c: prevent accessing watch-dog lag if head is NULL
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] GUIDINGLI commented on pull request #4466: sched/wdog/wd_start.c: move wd_expiration() function back to while loop
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466#issuecomment-913301481
I think Xiaoxiang is right, so can you update this PR and remove the:
if (((FAR struct wdog_s *)g_wdactivelist.head)->lag <= 0) in wd_expiration()
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] GUIDINGLI commented on pull request #4466: sched/wdog/wd_start.c: move wd_expiration() function back to while loop
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466#issuecomment-912441945
Can you explain why move the wd_expiration to while loop can fix the problem.
What's the difference ?
In wd_expiration(), will looping execute wdog->func from g_wdactivelist.
So wd_expiration() do once is enough, I think.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4466: sched/wdog/wd_start.c: move wd_expiration() function back to while loop
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4466:
URL: https://github.com/apache/incubator-nuttx/pull/4466#issuecomment-912894804
> > Can you explain why move the wd_expiration to while loop can fix the problem.
>
> @GUIDINGLI My uderstanding is that when wd_expiration() is outside the loop that it might be called even if wdog == NULL which might cause the hard fault. That´s the only difference I can see there. The tests seems ok but I am also not much confident about this as I haven´t worked with watchdogs before.
From this line: https://github.com/apache/incubator-nuttx/blob/master/sched/wdog/wd_start.c#L101-L110
```
/* Check if the watchdog at the head of the list is ready to run */
if (((FAR struct wdog_s *)g_wdactivelist.head)->lag <= 0)
{
/* Process the watchdog at the head of the list as well as any
* other watchdogs that became ready to run at this time
*/
while (g_wdactivelist.head &&
((FAR struct wdog_s *)g_wdactivelist.head)->lag <= 0)
```
Look like the if statement may access lag when head is NULL. Since while statement check the same condition, but verify head isn't NULL before the check, the simple fix is remove if statement.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org