You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "hujun260 (via GitHub)" <gi...@apache.org> on 2024/09/10 01:38:57 UTC
[PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
hujun260 opened a new pull request, #13349:
URL: https://github.com/apache/nuttx/pull/13349
## Summary
In SMP, we have encountered the situation many times where calling spin_lock_irqsave first and then calling enter_critical_section subsequently, this can lead to a deadlock.
Configuring NuttX and compile:
$ ./tools/configure.sh -l qemu-armv8a:nsh_smp
$ make
Running with qemu
$ qemu-system-aarch64 -cpu cortex-a53 -smp 4 -nographic \
-machine virt,virtualization=on,gic-version=3 \
-net none -chardev stdio,id=con,mux=on -serial chardev:con \
-mon chardev=con,mode=readline -kernel ./nuttx
## Impact
none
## Testing
ostest
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1765042687
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
The enter_critical_section relies on irq being enabled in its normal context
due to its specific strategy of reenable irq to respond to up_cpu_pause when the lock cannot be acquired immediately.
However, spin_lock_irqsave disabled irq make the strategy employed by enter_critical_section faild,
potentially leading to a deadlock situation."
cpu0</br> cpu1
enter_critical_section
up_cpu_pause(will wait until succed)
spin_lock_irqsave(NULL);
enter_critical_section(**deadlock**)
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1765042687
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
The enter_critical_section relies on irq being enabled in its normal context
due to its specific strategy of reenable irq to respond to up_cpu_pause when the lock cannot be acquired immediately.
However, spin_lock_irqsave disabled irq make the strategy employed by enter_critical_section faild,
potentially leading to a deadlock situation."
cpu0 cpu1
enter_critical_section<br/>up_cpu_pause(will wait until succed)<br/>
spin_lock_irqsave(NULL);<br/>
enter_critical_section(**deadlock**)<br/>
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1765042687
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
The enter_critical_section relies on irq being enabled in its normal context
due to its specific strategy of reenable irq to respond to up_cpu_pause when the lock cannot be acquired immediately.
However, spin_lock_irqsave disabled irq make the strategy employed by enter_critical_section faild,
potentially leading to a deadlock situation."
cpu0 cpu1
enter_critical_section<br\>
up_cpu_pause(will wait until succed)<br\>
spin_lock_irqsave(NULL);<br\>
enter_critical_section(**deadlock**)<br\>
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1765042687
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
The enter_critical_section relies on irq being enabled in its normal context
due to its specific strategy of reenable irq to respond to up_cpu_pause when the lock cannot be acquired immediately.
However, spin_lock_irqsave disabled irq make the strategy employed by enter_critical_section faild,
potentially leading to a deadlock situation."
cpu0         cpu1
enter_critical_section
up_cpu_pause(will wait until succed)
spin_lock_irqsave(NULL);
enter_critical_section(**deadlock**)
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1765042687
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
The enter_critical_section relies on irq being enabled in its normal context
due to its specific strategy of reenable irq to respond to up_cpu_pause when the lock cannot be acquired immediately.
However, spin_lock_irqsave disabled irq make the strategy employed by enter_critical_section faild,
potentially leading to a deadlock situation."
cpu0 cpu1
enter_critical_section
up_cpu_pause(will wait until succed)
spin_lock_irqsave(NULL);
enter_critical_section(**deadlock**)
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1765042687
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
The enter_critical_section relies on irq being enabled in its normal context
due to its specific strategy of reenable irq to respond to up_cpu_pause when the lock cannot be acquired immediately.
However, spin_lock_irqsave disabled irq make the strategy employed by enter_critical_section faild,
potentially leading to a deadlock situation."
cpu0<br/><br/><br/><br/><br/> cpu1
enter_critical_section
up_cpu_pause(will wait until succed)
spin_lock_irqsave(NULL);
enter_critical_section(**deadlock**)
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1765042687
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
The enter_critical_section relies on irq being enabled in its normal context
due to its specific strategy of reenable irq to respond to up_cpu_pause when the lock cannot be acquired immediately.
However, spin_lock_irqsave disabled irq make the strategy employed by enter_critical_section faild,
potentially leading to a deadlock situation."
cpu0 cpu1
enter_critical_section
up_cpu_pause(will wait until succed)
spin_lock_irqsave(NULL);
enter_critical_section(**deadlock**)
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1765042687
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
The enter_critical_section relies on irq being enabled in its normal context
due to its specific strategy of reenable irq to respond to up_cpu_pause when the lock cannot be acquired immediately.
However, spin_lock_irqsave disabled irq make the strategy employed by enter_critical_section faild,
potentially leading to a deadlock situation."
cpu0 cpu1
enter_critical_section<br/>up_cpu_pause(will wait until succed)<br/>
spin_lock_irqsave(NULL);<br/>
enter_critical_section(**deadlock**)<br/>
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "hujun260 (via GitHub)" <gi...@apache.org>.
hujun260 commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1765042687
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
The enter_critical_section relies on irq being enabled in its normal context
due to its specific strategy of reenable irq to respond to up_cpu_pause when the lock cannot be acquired immediately.
However, spin_lock_irqsave disabled irq make the strategy employed by enter_critical_section faild,
potentially leading to a deadlock situation."
cpu0 cpu1
enter_critical_section<br/>
up_cpu_pause(will wait until succed)<br/>
spin_lock_irqsave(NULL);<br>
enter_critical_section(**deadlock**)<br>
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #13349:
URL: https://github.com/apache/nuttx/pull/13349#discussion_r1764362955
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
Review Comment:
```suggestion
* During the OS startup phase, interrupts are masked until
```
Do not include any specific platform comments in NuttX Community FYI @acassis
##########
sched/irq/irq_csection.c:
##########
@@ -345,6 +345,26 @@ irqstate_t enter_critical_section(void)
}
else
{
+ /* CHECK IRQ
+ *
+ * After the OS starts, in the thread context,
+ * we must ensure that the interrupt is not masked
+ * when first enters the critical section.
+ * Otherwise, at this time when another CPU issues a pause
+ * interrupt to the current CPU, it can lead to a deadlock.
+ *
+ * During the vela os startup phase, interrupts are masked until
+ * the interrupt initialization function (irq_initialize) is
Review Comment:
Shouldn't we make sure that the common code flow before `irq_initialize()` calls spinlock instead of enter_critical_section, so that this check could be removed?
--
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
Re: [PR] irq: add up_irq_is_disabled to check deadlock [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 closed pull request #13349: irq: add up_irq_is_disabled to check deadlock
URL: https://github.com/apache/nuttx/pull/13349
--
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