This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
Re: PPP Deadlock
- From: Jonathan Larmour <jifl at eCosCentric dot com>
- To: John Paul King <john dot king at transdatainc dot com>
- Cc: ecos-patches at ecos dot sourceware dot org
- Date: Tue, 18 Jul 2006 18:21:48 +0100
- Subject: Re: PPP Deadlock
- References: <449DD117.7050401@transdatainc.com>
John Paul King wrote:
There is a deadlock wherein if cyg_pppd_main exits before
cyg_ppp_tx_thread starts, cyg_ppp_up or cyg_ppp_tx_thread will stall.
Thanks for the patch. Although I'm not intimately familiar with this code,
it seems to me that the logic of the expressions is the opposite sense from
what they should be. See specific comments below.
Index: net/ppp/current/src/sys-ecos.c
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/ppp/current/src/sys-ecos.c,v
retrieving revision 1.10
diff -u -5 -p -r1.10 sys-ecos.c
--- net/ppp/current/src/sys-ecos.c 23 Oct 2005 20:46:20 -0000 1.10
+++ net/ppp/current/src/sys-ecos.c 24 Jun 2006 23:17:15 -0000
@@ -556,11 +556,11 @@ static void cyg_ppp_tx_thread(CYG_ADDRWO
{
ppp_tty.tx_thread_running = true;
// Wait for the PPPD thread to get going and start the PPP
// initialization phase.
- while(phase == PHASE_DEAD )
+ while((phase == PHASE_DEAD) && ppp_tty.pppd_thread_running)
cyg_thread_delay(100);
Shouldn't this delay be while the pppd_thread is *not* running?
// Now loop until the link goes back down.
while( phase != PHASE_DEAD )
{
@@ -1700,25 +1700,26 @@ externC cyg_ppp_handle_t cyg_ppp_up( con
strncpy( devnam, devnam_arg, PATH_MAX );
ppp_tty.options = options;
+ cyg_semaphore_init( &ppp_tty.tx_sem, 0 );
+
// Start the PPPD thread
cyg_thread_create(CYGNUM_PPP_PPPD_THREAD_PRIORITY,
cyg_pppd_main,
(CYG_ADDRWORD)&ppp_tty,
"PPPD",
&cyg_pppd_stack[0],
CYGNUM_PPP_PPPD_THREAD_STACK_SIZE,
&ppp_tty.pppd_thread,
&cyg_pppd_thread_obj
);
+ ppp_tty.pppd_thread_running = true;
This seems unnecessary since it is set in sys_init() anyway.
cyg_thread_resume(ppp_tty.pppd_thread);
// Start the TX thread
- cyg_semaphore_init( &ppp_tty.tx_sem, 0 );
-
cyg_thread_create(CYGNUM_PPP_PPPD_THREAD_PRIORITY+1,
cyg_ppp_tx_thread,
(CYG_ADDRWORD)&ppp_tty,
"PPP Tx Thread",
&cyg_ppp_tx_thread_stack[0],
@@ -1728,11 +1729,11 @@ externC cyg_ppp_handle_t cyg_ppp_up( con
);
cyg_thread_resume(ppp_tty.tx_thread);
// Wait for the PPPD thread to get going and start the PPP
// initialization phase.
- while(phase == PHASE_DEAD )
+ while((phase == PHASE_DEAD) && ppp_tty.pppd_thread_running)
cyg_thread_delay(100);
Again, shouldn't this only be delaying as long as the pppd_thread is *not*
running?
Moving the semaphore init makes sense though. The pppd thread could be a
higher priority than the thread calling cyg_ppp_up. For now, that is the
only change I've checked in.
Thanks,
Jifl
--
eCosCentric http://www.eCosCentric.com/ The eCos and RedBoot experts
------["The best things in life aren't things."]------ Opinions==mine