Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interrupt lock missing in debug log functions #2652

Open
HiFiPhile opened this issue May 18, 2024 · 3 comments · May be fixed by #2653
Open

Interrupt lock missing in debug log functions #2652

HiFiPhile opened this issue May 18, 2024 · 3 comments · May be fixed by #2653

Comments

@HiFiPhile
Copy link
Collaborator

HiFiPhile commented May 18, 2024

What happened ?

Interrupt lock need to be added to log functions, calling from ISR could enter deadlock mostly for serial logging. #2649

Options:

  • Remove all logging from ISR.
  • Defer ISR logging, should be difficult with va_args.
  • Add interrupt lock, but macros need to be reworked since we don't know if they are called from device or host or typec stack in order to toggle interrupt accordingly.
  • Add a simple re-entrancy flag to skip printf.

Edit:
I must have bad memory, I met log racing while debugging dwc2 DMA with RTT but SEGGER_RTT_Write() is ISR-safe, it must happened inside libc printf().

How to reproduce ?

Run examples on ports who has log in ISR, like NRF5x.

@tlyu
Copy link
Contributor

tlyu commented May 18, 2024

I haven't had trouble with deadlocks with RTT on nRF52, compared to the Arduino core's Serial1.

On the other hand, anything with Bluetooth (Bluefruit lib; haven't tried anything lower level yet) seems to cause trouble with segfaults during debugging, including RTT.

@HiFiPhile
Copy link
Collaborator Author

I haven't had trouble with deadlocks with RTT on nRF52

I think it depends on timing (luck), implementation side they don't have interrupt lock.
https://github.com/NordicSemiconductor/nrfx/blob/4620e7ccfebc08dbd4c039bc7d530b62c4c68ca8/drivers/src/nrfx_uarte.c#L871

@HiFiPhile HiFiPhile linked a pull request May 19, 2024 that will close this issue
@ceedriic
Copy link

Options:

  • Remove all logging from ISR.
  • Defer ISR logging, should be difficult with va_args.

I think these two options are best.

In my own embedded code, if I really need to log in an IRQ, I use two custom functions that log either a single string or a string with a number, and these fonctions (1) don't use libc and (2) work "optimistically", (i.e. only if there is room in the output buffer). I think a general solution is difficult.

Also I don't think this is a real problem here for embedded systems, but in theory vfprintf() and friends can call malloc() and free() which is very bad practice in an IRQ....

If a tu_printf()-like function really wants to be called in an interrupt, then it would help if a different macro is defined (like tu_printf_irq()) and be overridable differently (CFG_TUSB_DEBUG_PRINTF vs CFG_TUSB_DEBUG_PRINTF_IRQ) so users can implement them in a different way. The context (irq or not) could be determined at runtime of course, but users could save code + IRQ latency by just defining CFG_TUSB_DEBUG_PRINTF_IRQ() a dummy statement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants