This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: Possible deadlock in serial.c
- From: Gary Thomas <gary at mlbassoc dot com>
- To: "David Marqvar (DAM)" <DAM at tt dot dk>
- Cc: eCos patches <ecos-patches at sources dot redhat dot com>
- Date: 21 May 2003 10:31:42 -0600
- Subject: Re: Possible deadlock in serial.c
- Organization: MLB Associates
- References: <E70B4FA554FB6C44BC7FAC9A9CE0CBC436D539@ntex.tt.dk>
On Wed, 2003-05-21 at 10:15, David Marqvar (DAM) wrote:
> In function
>
> static Cyg_ErrNo
> serial_get_config(cyg_io_handle_t handle, cyg_uint32 key, void
> *xbuf,
> cyg_uint32 *len)
> {
>
> Shouldn't the #ifdef and call to restart_rx( chan, false ); be added to the
> case below?
>
> case CYG_IO_GET_CONFIG_SERIAL_INPUT_FLUSH:
> // Flush any buffered input
> if (in_cbuf->len == 0) break; // Nothing to do if not buffered
> cyg_drv_mutex_lock(&in_cbuf->lock); // Stop any further input
> processing
> cyg_drv_dsr_lock();
> if (in_cbuf->waiting) {
> in_cbuf->abort = true;
> cyg_drv_cond_signal(&in_cbuf->wait);
> in_cbuf->waiting = false;
> }
> in_cbuf->get = in_cbuf->put = in_cbuf->nb = 0; // Flush buffered
> input
> cyg_drv_dsr_unlock();
> cyg_drv_mutex_unlock(&in_cbuf->lock);
> #ifdef CYGPKG_IO_SERIAL_FLOW_CONTROL
> restart_rx( chan, false );
> #endif
> break;
>
>
> If the serial input buffer is full, and the serial device has been told to
> stop rx, the serial driver should restart rx if the input buffer is flushed.
> Otherwise no data will never be received after that, thus no reads to the
> input buffer (through where the rx could be restarted) -> deadlock.
>
> Or?
>
> Will your changes to the serial driver (and the headerfile) be committed to
> the 2.0-branch too in a near future?
> (My suggestion)
>
> Please write back if I should file a patch for this.
This looks reasonable to me and I have applied the change (patch
attached).
I don't think that there will be any patches made to the 2.0 branch
now that it has been "released".
--
Gary Thomas <gary@mlbassoc.com>
MLB Associates
Index: io/serial/current/ChangeLog
===================================================================
RCS file: /misc/cvsfiles/ecos/packages/io/serial/current/ChangeLog,v
retrieving revision 1.51
diff -u -5 -p -r1.51 ChangeLog
--- io/serial/current/ChangeLog 25 Mar 2003 02:47:05 -0000 1.51
+++ io/serial/current/ChangeLog 21 May 2003 16:28:33 -0000
@@ -1,5 +1,11 @@
+2003-05-21 Gary Thomas <gary@mlbassoc.com>
+
+ * src/common/serial.c (serial_get_config): Restart receiver
+ after input queue flush if it had been throttled. Inspired
+ by David Marqvar <DAM@tt.dk>
+
2003-03-25 Jonathan Larmour <jifl@eCosCentric.com>
* src/common/serial.c (serial_get_config): For both INPUT_FLUSH
and OUTPUT_FLUSH keys, pass down to the hardware driver as well
to allow it to flush FIFOs.
Index: io/serial/current/src/common/serial.c
===================================================================
RCS file: /misc/cvsfiles/ecos/packages/io/serial/current/src/common/serial.c,v
retrieving revision 1.20
diff -u -5 -p -r1.20 serial.c
--- io/serial/current/src/common/serial.c 25 Mar 2003 16:10:48 -0000 1.20
+++ io/serial/current/src/common/serial.c 21 May 2003 16:28:35 -0000
@@ -622,10 +622,16 @@ serial_get_config(cyg_io_handle_t handle
(funs->set_config)(chan,
CYG_IO_SET_CONFIG_SERIAL_INPUT_FLUSH,
NULL, NULL);
cyg_drv_dsr_unlock();
cyg_drv_mutex_unlock(&in_cbuf->lock);
+#ifdef CYGPKG_IO_SERIAL_FLOW_CONTROL
+ // Restart receiver if it was shutdown
+ if ((chan->flow_desc.flags & CYG_SERIAL_FLOW_IN_THROTTLED) != 0) {
+ restart_rx( chan, false );
+ }
+#endif
break;
case CYG_IO_GET_CONFIG_SERIAL_ABORT:
// Abort any outstanding I/O, including blocked reads
// Caution - assumed to be called from 'timeout' (i.e. DSR) code