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/07/01 11:28:24 UTC
[GitHub] [incubator-nuttx] GUIDINGLI opened a new pull request #4020: SMP related small changes
GUIDINGLI opened a new pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020
## Summary
SMP related small changes
add [CPUx] to syslog
SMP should depends on ARCH_INTERRUPTSTACK
## Impact
## Testing
--
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] masayuki2009 commented on pull request #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#issuecomment-873317608
> > Thanks, you can push this PR directly.
>
> @xiaoxiang781216
>
> I noticed that we need to modify some files (e.g. arch/arm/src/armv6-m/arm_exception.S)
> So I will create a new PR tomorrow.
https://github.com/apache/incubator-nuttx/pull/4049
https://github.com/apache/incubator-nuttx/pull/4050
--
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 a change in pull request #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#discussion_r662700570
##########
File path: sched/Kconfig
##########
@@ -268,13 +268,25 @@ config SMP
default n
depends on ARCH_HAVE_MULTICPU
depends on ARCH_HAVE_TESTSET
+ depends on ARCH_INTERRUPTSTACK != 0
select SPINLOCK
select SCHED_RESUMESCHEDULER
select IRQCOUNT
---help---
Enables support for Symmetric Multi-Processing (SMP) on a multi-CPU
platform.
+ SMP mode must work with ARCH_INTERRUPTSTACK, here is the fail case:
+
+ CPU0 thread0 -> IRQ enter -> add thread0 to block_list -> IRQ leave(crash)
+ ||
+ /\
+ / \
+ CPU1 thread1 -> block_task -> take thread0 from block_list -> run thread0
+
+ CPU0 IRQ handler use thread0's stack, but thread0 may switch to CPU1, that
+ will caused IRQ handler stack curroption.
Review comment:
It's important to document this fact, but it's more important to express the fact by contract and then avoid the wrong config in the future. It's a good to add a select/depend ARCH_INTERRUPTSTACK here.
--
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 a change in pull request #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#discussion_r662926005
##########
File path: sched/Kconfig
##########
@@ -268,13 +268,25 @@ config SMP
default n
depends on ARCH_HAVE_MULTICPU
depends on ARCH_HAVE_TESTSET
+ depends on ARCH_INTERRUPTSTACK != 0
select SPINLOCK
select SCHED_RESUMESCHEDULER
select IRQCOUNT
---help---
Enables support for Symmetric Multi-Processing (SMP) on a multi-CPU
platform.
+ SMP mode must work with ARCH_INTERRUPTSTACK, here is the fail case:
+
+ CPU0 thread0 -> IRQ enter -> add thread0 to block_list -> IRQ leave(crash)
+ ||
+ /\
+ / \
+ CPU1 thread1 -> block_task -> take thread0 from block_list -> run thread0
+
+ CPU0 IRQ handler use thread0's stack, but thread0 may switch to CPU1, that
+ will caused IRQ handler stack curroption.
Review comment:
done
##########
File path: sched/Kconfig
##########
@@ -268,13 +268,25 @@ config SMP
default n
depends on ARCH_HAVE_MULTICPU
depends on ARCH_HAVE_TESTSET
+ depends on ARCH_INTERRUPTSTACK != 0
select SPINLOCK
select SCHED_RESUMESCHEDULER
select IRQCOUNT
---help---
Enables support for Symmetric Multi-Processing (SMP) on a multi-CPU
platform.
+ SMP mode must work with ARCH_INTERRUPTSTACK, here is the fail case:
Review comment:
done
--
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] masayuki2009 commented on pull request #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#issuecomment-873006405
> @masayuki2009 there are two places need to enable ARCH_INTERRUPTSTACK:
> modified: boards/arm/rp2040/raspberrypi-pico/configs/smp/defconfig
> modified: boards/arm/sam34/sam4cmp-db/configs/nsh/defconfig
@xiaoxiang781216
Yes, I will modify them later.
--
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] masayuki2009 commented on pull request #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#issuecomment-873044831
>Thanks, you can push this PR directly.
@xiaoxiang781216
I noticed that we need to modify some files (e.g. arch/arm/src/armv6-m/arm_exception.S)
So I will create a new PR tomorrow.
--
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 merged pull request #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020
--
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 #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#issuecomment-873069446
Sure.
--
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] gustavonihei commented on a change in pull request #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#discussion_r662523339
##########
File path: sched/Kconfig
##########
@@ -268,13 +268,25 @@ config SMP
default n
depends on ARCH_HAVE_MULTICPU
depends on ARCH_HAVE_TESTSET
+ depends on ARCH_INTERRUPTSTACK != 0
select SPINLOCK
select SCHED_RESUMESCHEDULER
select IRQCOUNT
---help---
Enables support for Symmetric Multi-Processing (SMP) on a multi-CPU
platform.
+ SMP mode must work with ARCH_INTERRUPTSTACK, here is the fail case:
+
+ CPU0 thread0 -> IRQ enter -> add thread0 to block_list -> IRQ leave(crash)
+ ||
+ /\
+ / \
+ CPU1 thread1 -> block_task -> take thread0 from block_list -> run thread0
+
+ CPU0 IRQ handler use thread0's stack, but thread0 may switch to CPU1, that
+ will caused IRQ handler stack curroption.
Review comment:
```suggestion
will caused IRQ handler stack corruption.
```
--
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 #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#issuecomment-872998382
@masayuki2009 there are two places need to enable ARCH_INTERRUPTSTACK:
modified: boards/arm/rp2040/raspberrypi-pico/configs/smp/defconfig
modified: boards/arm/sam34/sam4cmp-db/configs/nsh/defconfig
--
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 commented on a change in pull request #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#discussion_r662531394
##########
File path: sched/Kconfig
##########
@@ -268,13 +268,25 @@ config SMP
default n
depends on ARCH_HAVE_MULTICPU
depends on ARCH_HAVE_TESTSET
+ depends on ARCH_INTERRUPTSTACK != 0
select SPINLOCK
select SCHED_RESUMESCHEDULER
select IRQCOUNT
---help---
Enables support for Symmetric Multi-Processing (SMP) on a multi-CPU
platform.
+ SMP mode must work with ARCH_INTERRUPTSTACK, here is the fail case:
+
+ CPU0 thread0 -> IRQ enter -> add thread0 to block_list -> IRQ leave(crash)
+ ||
+ /\
+ / \
+ CPU1 thread1 -> block_task -> take thread0 from block_list -> run thread0
+
+ CPU0 IRQ handler use thread0's stack, but thread0 may switch to CPU1, that
+ will caused IRQ handler stack curroption.
Review comment:
I think the above information is important, but if ARCH_INTERRUPTSTACK is not it will not be seen. The information needs to be in the Documentation.
##########
File path: sched/Kconfig
##########
@@ -268,13 +268,25 @@ config SMP
default n
depends on ARCH_HAVE_MULTICPU
depends on ARCH_HAVE_TESTSET
+ depends on ARCH_INTERRUPTSTACK != 0
select SPINLOCK
select SCHED_RESUMESCHEDULER
select IRQCOUNT
---help---
Enables support for Symmetric Multi-Processing (SMP) on a multi-CPU
platform.
+ SMP mode must work with ARCH_INTERRUPTSTACK, here is the fail case:
Review comment:
```suggestion
N.B. SMP mode requires the use of ARCH_INTERRUPTSTACK
```
--
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] masayuki2009 commented on pull request #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#issuecomment-872659444
@GUIDINGLI
I noticed that some SMP configs do not set ARCH_INTERRUPTSTACK, so we must fix them first before merging this PR.
--
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 #4020: SMP should depends on ARCH_INTERRUPTSTACK
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4020:
URL: https://github.com/apache/incubator-nuttx/pull/4020#issuecomment-873008010
Thanks, you can push this PR directly.
--
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