You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mynewt.apache.org by Sterling Hughes <st...@gmail.com> on 2017/07/19 05:34:11 UTC
console_queue_char() in interrupt context
Hi,
As I was supporting the Ambiq series of processor, I ran into what I
think is a bug in console_queue_char().
Specifically,
- console_rx_char() is registered as a uart device callback, which
operates in interrupt context
- console_handle_char() is called by console_rx_char() in
uart_console.c.
- console_handle_char() calls console_out()
- console_out() calls write_char_cb() which is console_queue_char()
- console_queue_char() calls os_time_delay()
So, in the cases where console_queue_char() is called in interrupt
context, and os_time_delay() is hit, the operating system crashes.
os_time_delay() is not meant to be called from interrupt context.
I ran into this due to some errors managing the RX interrupt (more than
(n) bytes in a single RX interrupt), but I’m pretty sure the behavior
should not be this way/it will happen rarely. What do folks think in
terms of potential options for changing it?
Sterling
Re: console_queue_char() in interrupt context
Posted by Vipul Rahane <vi...@runtime.io>.
Hi,
I did look at this code recently. The way it is designed, without changing a lot of things, one way to avoid calling console_out() in interrupt context is to do it after a line is taken off the lines_queue in console_read(). However, it might get a bit complicated as there are multiple cases, I am not sure if console_read() would be the only consumer of a complete line but it surely looks like it should be since it is the only one that pulls out the line from the lines_queue.
Regards,
Vipul Rahane
> On Jul 18, 2017, at 10:34 PM, Sterling Hughes <st...@gmail.com> wrote:
>
> Hi,
>
> As I was supporting the Ambiq series of processor, I ran into what I think is a bug in console_queue_char().
>
> Specifically,
>
> - console_rx_char() is registered as a uart device callback, which operates in interrupt context
>
> - console_handle_char() is called by console_rx_char() in uart_console.c.
>
> - console_handle_char() calls console_out()
>
> - console_out() calls write_char_cb() which is console_queue_char()
>
> - console_queue_char() calls os_time_delay()
>
> So, in the cases where console_queue_char() is called in interrupt context, and os_time_delay() is hit, the operating system crashes. os_time_delay() is not meant to be called from interrupt context.
>
> I ran into this due to some errors managing the RX interrupt (more than (n) bytes in a single RX interrupt), but I’m pretty sure the behavior should not be this way/it will happen rarely. What do folks think in terms of potential options for changing it?
>
> Sterling
>
>
>
Re: console_queue_char() in interrupt context
Posted by Christopher Collins <ch...@runtime.io>.
On Wed, Jul 19, 2017 at 12:23:42PM -0400, David G. Simmons wrote:
>
> I've done a fair amount of digging around in the console in the past -- implementing the cursor, etc. -- and I don't recall the os_time_delay() from that, but it was admittedly a long time ago and I haven't looked at it recently. I'd be curious to figure out why the os_time_delay() is needed, and IF it is needed, in there at all. Simply taking it out may have some unintended consequences that may or may not be catastrophic.
The os_time_delay() gets called when a write to the UART driver fails
due to a full buffer. In this case, the code sleeps for one tick to let
the output buffer drain, then attempts to write the character again.
Chris
Re: console_queue_char() in interrupt context
Posted by "David G. Simmons" <sa...@mac.com>.
I've done a fair amount of digging around in the console in the past -- implementing the cursor, etc. -- and I don't recall the os_time_delay() from that, but it was admittedly a long time ago and I haven't looked at it recently. I'd be curious to figure out why the os_time_delay() is needed, and IF it is needed, in there at all. Simply taking it out may have some unintended consequences that may or may not be catastrophic.
Yeah, I know, not all that helpful. Sorry. I'll dig into it when I have a minute.
dg
> On Jul 19, 2017, at 1:34 AM, Sterling Hughes <st...@gmail.com> wrote:
>
> Hi,
>
> As I was supporting the Ambiq series of processor, I ran into what I think is a bug in console_queue_char().
>
> Specifically,
>
> - console_rx_char() is registered as a uart device callback, which operates in interrupt context
>
> - console_handle_char() is called by console_rx_char() in uart_console.c.
>
> - console_handle_char() calls console_out()
>
> - console_out() calls write_char_cb() which is console_queue_char()
>
> - console_queue_char() calls os_time_delay()
>
> So, in the cases where console_queue_char() is called in interrupt context, and os_time_delay() is hit, the operating system crashes. os_time_delay() is not meant to be called from interrupt context.
>
> I ran into this due to some errors managing the RX interrupt (more than (n) bytes in a single RX interrupt), but I’m pretty sure the behavior should not be this way/it will happen rarely. What do folks think in terms of potential options for changing it?
>
> Sterling
>
>
>
--
David G. Simmons
(919) 534-5099
Web <https://davidgs.com/> • Blog <https://davidgs.com/davidgs_blog> • Linkedin <http://linkedin.com/in/davidgsimmons> • Twitter <http://twitter.com/TechEvangelist1> • GitHub <http://github.com/davidgs>
/** Message digitally signed for security and authenticity.
* If you cannot read the PGP.sig attachment, please go to
* http://www.gnupg.com/ <http://www.gnupg.com/> Secure your email!!!
* Public key available at keyserver.pgp.com <http://keyserver.pgp.com/>
**/
♺ This email uses 100% recycled electrons. Don't blow it by printing!
There are only 2 hard things in computer science: Cache invalidation, naming things, and off-by-one errors.
Re: console_queue_char() in interrupt context
Posted by Christopher Collins <ch...@runtime.io>.
On Wed, Jul 19, 2017 at 09:00:16AM -0700, will sanfilippo wrote:
> I dont know enough about the console code to make an intelligent suggestion. There is no task associated with the console, right? If not, and the purpose of this is to wait some time maybe the only thing to do is set a timer to wake up and deal with this? Not sure that this would work though… And I dont think I have any other ideas :-)
I think this is caused by a recent change. It appears the console code
writes back to the uart in the middle of a read. Specifically:
int
console_handle_char(uint8_t byte)
{
/* ... */
/* Handle special control characters */
if (!isprint(byte)) {
handle_ansi(byte, input->line);
switch (byte) {
/* ... */
default:
insert_char(&input->line[cur], byte, end);
/* Falls through. */
case '\r':
/* Falls through. */
case '\n':
if (byte == '\n' && prev_endl == '\r') {
/* handle double end line chars */
prev_endl = byte;
break;
}
prev_endl = byte;
input->line[cur + end] = '\0';
console_out('\r'); // <--- ***
console_out('\n'); // <--- ***
cur = 0;
end = 0;
os_eventq_put(lines_queue, ev);
It seems the console is normalizing line endings. Regardless of what
type of newline is read, it always outputs "\r\n".
I don't know what the solution is, but Sterling is correct that the ISR
shouldn't be performing a UART write.
Chris
>
>
> > On Jul 18, 2017, at 10:34 PM, Sterling Hughes <st...@gmail.com> wrote:
> >
> > Hi,
> >
> > As I was supporting the Ambiq series of processor, I ran into what I think is a bug in console_queue_char().
> >
> > Specifically,
> >
> > - console_rx_char() is registered as a uart device callback, which operates in interrupt context
> >
> > - console_handle_char() is called by console_rx_char() in uart_console.c.
> >
> > - console_handle_char() calls console_out()
> >
> > - console_out() calls write_char_cb() which is console_queue_char()
> >
> > - console_queue_char() calls os_time_delay()
> >
> > So, in the cases where console_queue_char() is called in interrupt context, and os_time_delay() is hit, the operating system crashes. os_time_delay() is not meant to be called from interrupt context.
> >
> > I ran into this due to some errors managing the RX interrupt (more than (n) bytes in a single RX interrupt), but I’m pretty sure the behavior should not be this way/it will happen rarely. What do folks think in terms of potential options for changing it?
> >
> > Sterling
> >
> >
> >
>
Re: console_queue_char() in interrupt context
Posted by will sanfilippo <wi...@runtime.io>.
I dont know enough about the console code to make an intelligent suggestion. There is no task associated with the console, right? If not, and the purpose of this is to wait some time maybe the only thing to do is set a timer to wake up and deal with this? Not sure that this would work though… And I dont think I have any other ideas :-)
> On Jul 18, 2017, at 10:34 PM, Sterling Hughes <st...@gmail.com> wrote:
>
> Hi,
>
> As I was supporting the Ambiq series of processor, I ran into what I think is a bug in console_queue_char().
>
> Specifically,
>
> - console_rx_char() is registered as a uart device callback, which operates in interrupt context
>
> - console_handle_char() is called by console_rx_char() in uart_console.c.
>
> - console_handle_char() calls console_out()
>
> - console_out() calls write_char_cb() which is console_queue_char()
>
> - console_queue_char() calls os_time_delay()
>
> So, in the cases where console_queue_char() is called in interrupt context, and os_time_delay() is hit, the operating system crashes. os_time_delay() is not meant to be called from interrupt context.
>
> I ran into this due to some errors managing the RX interrupt (more than (n) bytes in a single RX interrupt), but I’m pretty sure the behavior should not be this way/it will happen rarely. What do folks think in terms of potential options for changing it?
>
> Sterling
>
>
>