wangchdo commented on PR #17018: URL: https://github.com/apache/nuttx/pull/17018#issuecomment-3356868116
> > > > The current syslog will alway log by rawstream, which will always firstly put the message to a buffer and then flush them by calling syslog_write(). syslog_write() will alway broadcast them to all channels. There are some drawbacks: > > > > ``` > > > > 1. The rawstream buffer is allocated in the caller's > > > > stack as temperory variable, which cause huge stack usage, > > > > this is not good for the platform wit limited resource > > > > ``` > > > > > > > > > is 64byes buffer huge? > > > > ``` > > > > 2. In interrupt context, syslog() will firstly put the message > > > > in a global buffer, using a spinlock to protect it, which will > > > > cause dead lock in some cases in multicore system, when > > > > doing syslog in exception context from interrupt > > > > ``` > > > > > > > > > without this buffer, the interrupt log will make the normal log unreadable. Anyway, you can disable interrupt buffer if you don't want. > > > > ``` > > > > 3. syslog_write() will always broadcast to all the channels, > > > > so it can only be used when all the channels are ready, making > > > > syslog() not able to be used at the early age during boot. > > > > ``` > > > > > > > > > but the channel which isn't ready to log, should skip the action by calling https://github.com/apache/nuttx/blob/master/include/nuttx/init.h#L43-L48. > > > > ``` > > > > 4. with some options enabled syslog() relies on kernel to be ready > > > > before it can be used, for example getting kernel > > > > state or timestamp and put them with the message, this will also make > > > > syslog() not able to work before kernel is ready. > > > > ``` > > > > > > > > > nx_vsyslog already check the initialize state to avoid calling these functions too earlier. > > > > This patch introduced LOG_CONSOLE_DEBUG priority to syslog(), which is implemented in a light-weight way: no buffering, unicast and no relying on kernel is ready. Calling syslog() with this priority the message will always be sent to console in a synchronous way, and users can use it as long as uart early initializing is completed, which is before nx_start() is called. > > > > > > > > > basically, this api is used for early log, so it's better to contain early in the function to make the usage clear. > > > > > > Hi @xiaoxiang781216 > > I think this new function added to syslog is not only for early stage, as I put in the commit message, it is for: > > ``` > > 1. uinicast(log to serial) > > ``` > > As i said before, you can achieve the goal by only enabling the default syslog channel. Why do you need add additional code do the same thing? > > > ``` > > 2. synchronous > > ``` > > Why do you think the default implementation isn't synchronous? could you explain more what your meaning of synchronous? > > > ``` > > 3. light-weight (no buffering, no spinlock, no flushing) > > ``` > > without buffer and spinlock, multi thread output will mix in the same line which make the output unreadable, so your implementation is only useful in the very early stage before OS scheduler start working. > > > ``` > > 4. work at anytime(after earlyserialint() is completed), > > ``` > > the default implementation with the default/ramlog channel work at anytime too. Basically, whether syslog can work in early stage or special context majorly depend on the implementation of backend channel, not the frontend. > > > ``` > > 5. work at anywhere(task, interrupt, crash context, interrut nest task, crash nest interrup ....) > > ``` > > the default implementation work in any context too. > > > So I think `LOG_LOWPUT `should be a better option, I just amended the patch updating `LOG_CONSOLE_DEBUG `to `LOG_LOWPUT` HI @xiaoxiang781216 I believe we should add this new front-end function to syslog(), in order to support the following behaviors: ``` 1. Lightweight: users can be assured that syslog will only perform formatted output of the provided input string 2. No buffering in either task or interrupt context 3. No spinlock or critical section protection, ensuring there is no risk of deadlock or crash caused by these mechanisms (users must handle protection themselves if needed) 4. Output directed only to `up_putc()` ``` If we attempt to achieve these by modifying the existing backend implementation, it would have two drawbacks: ``` 1. It could make syslog more difficult for users to use. 2. We would lose the benefits of the current default syslog, such as buffering, spinlock protection, and the additional debug information it provides ```. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
