You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "chirping78 (via GitHub)" <gi...@apache.org> on 2023/09/07 01:18:29 UTC
[GitHub] [nuttx] chirping78 opened a new pull request, #10496: smp: irq_csection: enable irq to process the pending pause request
chirping78 opened a new pull request, #10496:
URL: https://github.com/apache/nuttx/pull/10496
## Summary
It's found in a complicated task-exiting/rescheduling scenario, two CPUs are deadlocked.
Use JTAG to dump the backtrace of the two CPUs to see what happened.
CPU0‘s backtrace shows it's waiting CPU1 to pause:
```
(gdb) bt
#0 spin_lock (lock=lock@entry=0x40ed66cd <g_cpu_paused+1> "\001") at semaphore/spinlock.c:74
#1 0x40017bb0 in up_cpu_pause (cpu=cpu@entry=1) at armv7-a/arm_cpupause.c:280
#2 0x40066714 in nxsched_add_readytorun (btcb=btcb@entry=0x418b9b50) at sched/sched_addreadytorun.c:260
#3 0x400686f0 in nxsem_post (sem=sem@entry=0x42a905bc) at semaphore/sem_post.c:174
#4 0x40065e58 in pthread_sem_give (sem=sem@entry=0x42a905bc) at pthread/pthread_initialize.c:108
#5 0x4006603c in pthread_notifywaiters (pjoin=0x42a905b0) at pthread/pthread_completejoin.c:76
#6 pthread_completejoin (pid=<optimized out>, exit_value=exit_value@entry=0x0) at pthread/pthread_completejoin.c:224
#7 0x401c6f40 in nx_pthread_exit (exit_value=exit_value@entry=0x0) at pthread/pthread_exit.c:82
#8 0x40093b7c in pthread_exit (exit_value=0x0) at pthread/pthread_exit.c:71
#9 0x40093acc in pthread_startup (entry=<optimized out>, arg=<optimized out>) at pthread/pthread_create.c:59
#10 0x401c6be4 in pthread_start () at pthread/pthread_create.c:139
#11 0x00000000 in ?? ()
```
And the backtrace shows that CPU1 is in `enter_critical_section()` and has percepted it is asked to pause:
```
(gdb) bt
#0 enter_critical_section () at irq/irq_csection.c:320
#1 0x40067b98 in nxsem_wait (sem=0x418a7a00) at semaphore/sem_wait.c:88
#2 sem_wait (sem=sem@entry=0x418a7a00) at semaphore/sem_wait.c:303
#3 0x4009315c in nxmutex_lock (mutex=mutex@entry=0x418a7a00) at misc/lib_mutex.c:203
#4 0x400a37b4 in mm_lock (heap=heap@entry=0x418a7a00) at mm_heap/mm_lock.c:96
#5 0x400a3b58 in mm_free (heap=0x418a7a00, mem=0x43680000) at mm_heap/mm_free.c:96
#6 0x400a1764 in free (mem=<optimized out>) at umm_heap/umm_free.c:49
```
So it's repeatly looping, trying to ack the pause request, a SGI interrupt.
But for some reason, the local interrupt on CPU1 is already disabled, so CPU1 will never ack the SGI interrupt.
This is why the two CPUs are deadlocked.
```
try_again:
ret = up_irq_save();
...
...
up_irq_restore(ret);
goto try_again;
```
This fix will really enable local interrupt when doing the retry.
## Impact
smp, `enter_critical_section`
## Testing
Tested in an armv7a smp configuration.
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1768002498
@chirping78
>The target is a dual core cortex A7.
Since I do not have a dual Cortex-A7-based board, I tested the modified smp app with NXP sabre-6quad (quad Cortex-A9) with NCPUS=2. (I modified sabre-6quad:smp config file)
>About the .config, I take a glance on the file and found some sensitive information, like internal components and partner name. There maybe some discipline prevent me to post it here.
Can you remove such sensitive information and post the .config file?
> Very soon, in seconds.
Hmm, I've never reproduced the deadlock with the modified smp app on the board.
I repeated running the smp app more than 1000 times. (It took more than 5 hours)
Now, I've been testing the app on different CPUs such as esp32-devkitc but I still can not reproduce the 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
[GitHub] [nuttx] chirping78 closed pull request #10496: smp: irq_csection: enable irq to process the pending pause request
Posted by "chirping78 (via GitHub)" <gi...@apache.org>.
chirping78 closed pull request #10496: smp: irq_csection: enable irq to process the pending pause request
URL: https://github.com/apache/nuttx/pull/10496
--
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] [nuttx] chirping78 commented on pull request #10496: smp: irq_csection: enable irq to process the pending pause request
Posted by "chirping78 (via GitHub)" <gi...@apache.org>.
chirping78 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1716890752
> @chirping78
>
> > But for some reason, the local interrupt on CPU1 is already disabled, so CPU1 will never ack the SGI interrupt.
>
> Can you identify where the local interrupt on cpu1 was disabled when the deadlock happened?
>
> Also, I’d like to know that what happens if the DEBUGASSERTION is enabled.
Need get time and the board to reproduce.
Will return here after I get the information.
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "chirping78 (via GitHub)" <gi...@apache.org>.
chirping78 closed pull request #10496: smp: irq_csection: enable irq to process the pending pause request
URL: https://github.com/apache/nuttx/pull/10496
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1763419169
> > This operation causes a deadlock because of the critical section of [`esp32s3_phy_enable`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/esp32s3/esp32s3_wireless.c#L468). The CPU pause will hang [here](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/common/xtensa_cpupause.c#L196): not because of the CPU pause operation (the other CPU was already resumed), but because this `enter_critical_section` will keep looping in [`irq_waitlock`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/sched/irq/irq_csection.c#L123) because it can't acquire the spinlock (owned by `esp32s3_phy_enable`).
>
> Do you handle up_cpu_pausereq in arch/chip specific code correctly?
[`up_cpu_pausereq`](https://github.com/apache/nuttx/blob/35b597ec2c0154ca675d2cd73f969c717aa3d739/arch/xtensa/src/common/xtensa_cpupause.c#L67) is a common code of xtensa arch, which is the same as other archs. This is check in [`xtensa_pause_handler`](https://github.com/apache/nuttx/blob/35b597ec2c0154ca675d2cd73f969c717aa3d739/arch/xtensa/src/common/xtensa_cpupause.c#L187) which is also similar/equal to other archs ([`arm_pause_handler`](https://github.com/apache/nuttx/blob/35b597ec2c0154ca675d2cd73f969c717aa3d739/arch/arm/src/armv7-a/arm_cpupause.c#L197), for instance: both are called inside an ISR)
I can't see any problems regarding handling. I'd say that the same situation will happen for any arch: the problem seems to be calling `up_cpu_pause` on a nested critical section.
Do you have any thoughts about it, @xiaoxiang781216 ?
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "chirping78 (via GitHub)" <gi...@apache.org>.
chirping78 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1767947214
> Could you tell us more about the target information? (CPU type and the number of CPUs and so on) I'd like to see the .config file.
The target is a dual core cortex A7.
About the .config, I take a glance on the file and found some sensitive information, like internal components and partner name. There maybe some discipline prevent me to post it here.
>
> Also, please provide the following information
>
> * the toolchain you are using
The toolchain version is:
````
Target: arm-none-eabi
Configured with: /data/jenkins/workspace/GNU-toolchain/arm-11/src/gcc/configure --target=arm-none-eabi --prefix=/data/jenkins/workspace/GNU-toolchain/arm-11/build-arm-none-eabi/install --with-gmp=/data/jenkins/workspace/GNU-toolchain/arm-11/build-arm-none-eabi/host-tools --with-mpfr=/data/jenkins/workspace/GNU-toolchain/arm-11/build-arm-none-eabi/host-tools --with-mpc=/data/jenkins/workspace/GNU-toolchain/arm-11/build-arm-none-eabi/host-tools --with-isl=/data/jenkins/workspace/GNU-toolchain/arm-11/build-arm-none-eabi/host-tools --disable-shared --disable-nls --disable-threads --disable-tls --enable-checking=release --enable-languages=c,c++,fortran --with-newlib --with-gnu-as --with-gnu-ld --with-sysroot=/data/jenkins/workspace/GNU-toolchain/arm-11/build-arm-none-eabi/install/arm-none-eabi --with-multilib-list=aprofile,rmprofile --with-pkgversion='Arm GNU Toolchain 11.3.Rel1' --with-bugurl=https://bugs.linaro.org/
Thread model: single
Supported LTO compression algorithms: zlib
gcc version 11.3.1 20220712 (Arm GNU Toolchain 11.3.Rel1)
````
> * how long does it take to reproduce the deadlock with the modified smp app
Very soon, in seconds.
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "chirping78 (via GitHub)" <gi...@apache.org>.
chirping78 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1751691440
> @chirping78
>
> > But for some reason, the local interrupt on CPU1 is already disabled, so CPU1 will never ack the SGI interrupt.
>
> Can you identify where the local interrupt on cpu1 was disabled when the deadlock happened?
>
> Also, I’d like to know that what happens if the DEBUGASSERTION is enabled.
I got the board today, and can reproduce the issue w/o this PR change.
About "where the local interrupt on cpu1 was disabled", I can't find a proper way to find this answer. Could you sugguest a method, maybe gdb script or log? @masayuki2009
About `DEBUGASSERTION`, I found `CONFIG_DEBUG_ASSERTIONS` is opened:
````
nuttx$ grep DEBUG_ .config|grep ASS
CONFIG_DEBUG_ASSERTIONS=y
````
but it does not print assert when this issue happend.
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1763598273
>Since I can't post the vendor code here, I modified the smp tesing code to simulate the scenario.
>https://github.com/chirping78/nuttx-apps/commit/c6dbb0aa0d43d16e636954518b1eb4d6a8f67804
>You can try this modified smp testing on a smp board in case you have, to see whether it can reproduce?
@chirping78
Could you tell us more about the target information? (CPU type and the number of cores and so on)
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1763059015
> This operation causes a deadlock because of the critical section of [`esp32s3_phy_enable`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/esp32s3/esp32s3_wireless.c#L468). The CPU pause will hang [here](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/common/xtensa_cpupause.c#L196): not because of the CPU pause operation (the other CPU was already resumed), but because this `enter_critical_section` will keep looping in [`irq_waitlock`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/sched/irq/irq_csection.c#L123) because it can't acquire the spinlock (owned by `esp32s3_phy_enable`).
>
Do you handle up_cpu_pausereq in arch/chip specific code correctly?
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "chirping78 (via GitHub)" <gi...@apache.org>.
chirping78 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1797900084
Have asked the chip vendor to remove their wrong `up_irq_save` usage, so this PR change is not needed any more. Close it.
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1751699847
@chirping78 please rebase your change to the last master, ci break is fixed sometime ago.
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "chirping78 (via GitHub)" <gi...@apache.org>.
chirping78 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1758804497
I tried to print the cpu/pid of the task which has disabled IRQ with following changed `up_irq_save`.
Please note that:
- the changed part is closed in `#if 1` and `#endif`
- can not use `syslog` / `info`, which will call `up_irq_save` recursively. So `up_putc` is used here
- can not use `up_interrupt_context` / `gettid`. They call `up_irq_save` recursively. So copy their logic here (the IRQ is disabled)
```
irqstate_t up_irq_save(void)
{
unsigned int cpsr;
#if 1
int cpu;
pid_t tid;
static pid_t printed_pid[2] = {0};
#endif
__asm__ __volatile__
(
"\tmrs %0, cpsr\n"
"\tcpsid i\n"
#if defined(CONFIG_ARMV7A_DECODEFIQ)
"\tcpsid f\n"
#endif
: "=r" (cpsr)
:
: "memory"
);
#if 1
if (g_nx_initstate >= OSINIT_IDLELOOP && CURRENT_REGS == NULL)
{
cpu = up_cpu_index();
struct tcb_s *rtcb = current_task(cpu);
tid = rtcb->pid;
if (!(cpsr & 0x80))
{
if (tid != printed_pid[cpu])
{
up_putc(cpu + '0');
//up_putc(':');
up_putc((tid / 10) + '0');
up_putc((tid % 10) + '0');
//up_putc(':');
//up_putc('0');
up_putc('\n');
printed_pid[cpu] = tid;
}
}
}
#endif
return cpsr;
}
```
The bad news is, with the change this issue can not be reproduced, although the change has been limit to minimum range.
Any other suggestion? @masayuki2009
--
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] [nuttx] pkarashchenko commented on a diff in pull request #10496: smp: irq_csection: enable irq to process the pending pause request
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#discussion_r1318291896
##########
sched/irq/irq_csection.c:
##########
@@ -355,7 +355,8 @@ irqstate_t enter_critical_section(void)
* request.
*/
- up_irq_restore(ret);
+ up_irq_enable();
+ up_irq_save();
Review Comment:
@masayuki2009 could you 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] [nuttx] masayuki2009 commented on a diff in pull request #10496: smp: irq_csection: enable irq to process the pending pause request
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on code in PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#discussion_r1318381763
##########
sched/irq/irq_csection.c:
##########
@@ -355,7 +355,8 @@ irqstate_t enter_critical_section(void)
* request.
*/
- up_irq_restore(ret);
+ up_irq_enable();
+ up_irq_save();
Review Comment:
I'll try this PR 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] [nuttx] masayuki2009 commented on pull request #10496: smp: irq_csection: enable irq to process the pending pause request
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1712370152
@chirping78
>But for some reason, the local interrupt on CPU1 is already disabled, so CPU1 will never ack the SGI interrupt.
Can you identify where the local interrupt on cpu1 was disabled when the deadlock happened?
Also, I’d like to know that what happens if the DEBUGASSERTION is enabled.
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1762851740
> You can see that the problem is caused by mixed using `up_irq_save` and `enter_critical_section`. Is this a permitted pattern?
>
> * if it is, then this PR is needed to fix up.
> * if it isn't, the this PR is not needed. We must use only `enter_critical_section`, instead of `up_irq_save` in smp case?
Just adding some information on this topic:
I've found a similar issue with ESP32-S3 + SMP in a WIP development regarding a function that performs an SPI flash operation in [`esp32s3_phy_enable`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/esp32s3/esp32s3_wireless.c#L455)
Basically, this function will call (WIP) another function to read data from SPI flash. This operation requires disabling the non-IRAM interrupts on both cores in [`spiflash_start`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/esp32s3/esp32s3_spiflash.c#L261). This function [posts to a semaphore](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/esp32s3/esp32s3_spiflash.c#L289) to make a pinned-to-the-other-core task to make it run a busy loop while the operation takes place. [`nxsem_post`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/sched/semaphore/sem_post.c#L70) calls [`nxsched_add_readytorun`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/sched/sched/sched_addreadytorun.c#L152) which will [pause the other CPU](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/sched/sched/sched_addreadytorun.c#
L260) to set up the `g_assignedtasks` on the other core and add the task that performs the busy loop.
This operation causes a deadlock because of the critical section of [`esp32s3_phy_enable`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/esp32s3/esp32s3_wireless.c#L468). The CPU pause will hang [here](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/common/xtensa_cpupause.c#L196): not because of the CPU pause operation (the other CPU was already resumed), but because this `enter_critical_section` will keep looping in [`irq_waitlock`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/sched/irq/irq_csection.c#L123) because it can't acquire the spinlock (owned by `esp32s3_phy_enable`).
This situation is avoided by not entering a critical section in [`esp32s3_phy_enable`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/arch/xtensa/src/esp32s3/esp32s3_wireless.c#L468). Using `spin_lock_irqsave(NULL)` fixes the issue.
Please note that [`spin_lock_irqsave`](https://github.com/apache/nuttx/blob/1989749850eff1b57bf543983ddffbdc8307d071/sched/irq/irq_spinlock.c#L85) calls `up_irq_save`, so I'd say that calling it is not an issue to perform a CPU pause operation, the problem is performing this operation in a nested critical section (called by `enter_critical_section`).
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1758833667
> The bad news is, with the change this issue can not be reproduced, although the change has been limit to minimum range.
@chirping78
I think you should not use `up_putc()` because it's too slow. Instead, you should use trace buffers.
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "chirping78 (via GitHub)" <gi...@apache.org>.
chirping78 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1761498868
@masayuki2009
Since when the issue is reproduced, the system will hang and can't use `trace dump` command, so I made a homemade trace buffer to save `up_irq_save` / `up_irq_restore` call.
And after the issue is reproduced, I use jtag to dump the trace buffer.
Then I really find something in the trace: there are some directly `up_irq_save` / `up_irq_restore` call in the chip vendor code.
Since I can't post the vendor code here, I modified the smp tesing code to simulate the scenario.
https://github.com/chirping78/nuttx-apps/commit/c6dbb0aa0d43d16e636954518b1eb4d6a8f67804
You can try this modified smp testing on a smp board in case you have, to see whether it can reproduce?
You can see that the problem is caused by mixed using `up_irq_save` and `enter_critical_section`. Is this a permitted pattern?
- if it is, then this PR is needed to fix up.
- if it isn't, the this PR is not needed. We must use only `enter_critical_section`, instead of `up_irq_save` in smp case?
--
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] smp: irq_csection: enable irq to process the pending pause request [nuttx]
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1752901929
@chirping78
>I got the board today, and can reproduce the issue w/o this PR change.
Can you attach .config to this PR?
>About "where the local interrupt on cpu1 was disabled", I can't find a proper way to find this answer. Could you sugguest a method, maybe gdb script or log? @masayuki2009
You need to modify up_irq_save() to find where the local interrupt was disabled last time.
Please note that up_irq_save() is called on both CPUs and is also called recursively.
--
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] [nuttx] chirping78 commented on pull request #10496: smp: irq_csection: enable irq to process the pending pause request
Posted by "chirping78 (via GitHub)" <gi...@apache.org>.
chirping78 commented on PR #10496:
URL: https://github.com/apache/nuttx/pull/10496#issuecomment-1722660549
Was closed by accident when syncing my repository. Restore it now.
--
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