This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PPP Deadlock


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]