xiaoxiang781216 commented on code in PR #14695:
URL: https://github.com/apache/nuttx/pull/14695#discussion_r1836102937
##########
drivers/syslog/Kconfig:
##########
@@ -267,19 +267,25 @@ config SYSLOG_STREAM
config SYSLOG_CONSOLE
bool "Log to /dev/console"
- default !ARCH_LOWPUTC && !SYSLOG_CHAR && !RAMLOG_SYSLOG &&
!SYSLOG_RPMSG && !SYSLOG_RTT
+ default !SYSLOG_CHAR && !RAMLOG_SYSLOG && !SYSLOG_RPMSG && !SYSLOG_RTT
Review Comment:
> > > > syslog can be called from the interrupt context must been keep as
before
> > >
> > >
> > > why are you pretending like we have a working implementation of syslog
which meets your requirements?
> >
> >
> > In production, we normally use ramlog or rpmsg syslog. In development,
we use the default syslog with interrupt buffer.
>
> you just don't care occasional deadlock during development? or, you are
lucky and have not been suffered by the issue?
We don't hit the deadlock since the driver already do the protection.
>
> > > because you don't care SMP?
> >
> >
> > No, we care SMP a lot (actually, all recent SMP improvement come from
us, and more will come in), since we already chip more than 400 million
products.
> > > > if so, do you plan to remove the capability that sem can be posted
and mqueue can be sent from interrupt handler?
> > >
> > >
> > > no. (at least for now.)
> > > this PR just disables a functionality which is broken. (just because
leaving it enabled hurts users. i received complaints from users.) if you want
to use it and you don't care the breakage, you can still enable it.
> >
> >
> > But the syslog console which can't be called from panic/assert/interrupt
need be fixed or more another approach is provided before we merge this patch.
>
> why? you can use ramlog or whatever as you said, can't you?
>
Yes, I can change to ramlog in my defconfig, and you can change to console
in your defconfig too, but you can't change the default setting in master
branch since all defconfig expect syslog can be called from
panic/assert/interrupt.
> > > > but panic/assert need work in production.
> > >
> > >
> > > it's a valid concern and i already proposed a possible solution
earlier in this thread: [#14695
(comment)](https://github.com/apache/nuttx/pull/14695#discussion_r1833814177)
> >
> >
> > But we need an implementation, not just a proposal before switching.
>
> your response to my proposal was just "panic is just one case; driver may
output log in interrupt too." i thought you meant the approach was not
acceptable at all for you. what's the point to write an implementation just to
be dismissed?
I don't oppose this patch, but all current syslog in kernel need migrate to
the interrupt safe approach before we merge this one.
--
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]
