This is the mail archive of the
ecos-devel@sourceware.org
mailing list for the eCos project.
Re: RFC: bsd_tcpip patch on synch.c
- From: Lambrecht Jürgen <J dot Lambrecht at TELEVIC dot com>
- To: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- Cc: "ecos-devel at ecos dot sourceware dot org" <ecos-devel at ecos dot sourceware dot org>
- Date: Thu, 27 Jun 2013 16:20:36 +0200
- Subject: Re: RFC: bsd_tcpip patch on synch.c
- References: <51CC0714 dot 6040805 at televic dot com>,<DUB124-W44EE5E989F221552E035FDE4750 at phx dot gbl> <DUB124-W44538C6E274AF947C90B72E4750 at phx dot gbl>
On 06/27/2013 02:51 PM, Bernd Edlinger wrote:
> Hi Jürgen,
>
>
> the variant with cyg_mutex_trylock is like busy waiting, and
> should be reverted. But I agree that the
> "while (!cyg_mutex_lock()) continue;"
> construct is somehow really bad style.
>
> I do not think that cyg_mutex_lock can ever return false,
> unless the spl_mutex is completely invalid.
After digging deep, I found that our problem was caused by something
else, so this fix is not needed, but we kept it because we did not like
that construct as you also point out.
>
> If you really expect cyg_mutex_lock to ever return false,
> then the right thing to do would be to assert(false)
> and print a callstack or directly enter the debugger.
No problem anymore..
But is there a better way to loop over the mutex lock?
Kind regards,
Jürgen
>
>
> Regards
> Bernd Edlinger.
>
>
>> From: J.Lambrecht@TELEVIC.com
>> To: ecos-devel@ecos.sourceware.org
>> CC: bernd.edlinger@hotmail.de
>> Date: Thu, 27 Jun 2013 11:34:12 +0200
>> Subject: RFC: bsd_tcpip patch on synch.c
>>
>> Hello,
>>
>> The patch below we already use since 2009. But I did not code the patch,
>> so I do not know what problem it solves. Here are our CVS logs about the
>> patch:
>>
>> - To avoid deadlock on mutex 'splx_mutex', I changed 'cyg_mutex_lock'
>> into 'cyg_mutex_trylock'.
>> - Add 'trylock' and 'cyg_thread_delay' to spl_any() in order to handle
>> the deadlock issue on the mutex 'splx_mutex'
>>
>> I am applying the patches from Bernd Edlinger
>> (http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001656)(to fix an
>> anoying problem "packet loss every 20 minutes when ARP timeout expires"
>> and to add raw packets).
>> And I wonder if this patch (below) is still valid?
>> --------------------------------------------------------------------------------
>> Index: net/bsd_tcpip/current/src/ecos/synch.c
>> ===================================================================
>> RCS file: /cvs/ecos/ecos-opt/net/net/bsd_tcpip/current/src/ecos/synch.c,v
>> retrieving revision 1.3
>> diff -u -5 -p -r1.3 synch.c
>> --- net/bsd_tcpip/current/src/ecos/synch.c 29 Jan 2009 17:49:56
>> -0000 1.3
>> +++ net/bsd_tcpip/current/src/ecos/synch.c 27 Jun 2013 09:12:54 -0000
>> @@ -115,12 +115,14 @@ static volatile cyg_handle_t splx_thread
>> static inline cyg_uint32
>> spl_any( cyg_uint32 which )
>> {
>> cyg_uint32 old_spl = spl_state;
>> if ( cyg_thread_self() != splx_thread ) {
>> - while ( !cyg_mutex_lock( &splx_mutex ) )
>> + while ( !cyg_mutex_trylock( &splx_mutex ) ) {
>> + cyg_thread_delay(1);
>> continue;
>> + }
>> old_spl = 0; // Free when we unlock this context
>> CYG_ASSERT( 0 == splx_thread, "Thread still owned" );
>> CYG_ASSERT( 0 == spl_state, "spl still set" );
>> splx_thread = cyg_thread_self();
>> }
>> --------------------------------------------------------------------------------
>> Kind regards,
>> Jürgen
>>
>> --
>> Jürgen Lambrecht
>> R&D Associate
>> Mobile: +32 499 644 531
>> Tel: +32 (0)51 303045 Fax: +32 (0)51 310670
>> http://www.televic-rail.com
>> Televic Rail NV - Leo Bekaertlaan 1 - 8870 Izegem - Belgium
>> Company number 0825.539.581 - RPR Kortrijk
--
Jürgen Lambrecht
R&D Associate
Mobile: +32 499 644 531
Tel: +32 (0)51 303045 Fax: +32 (0)51 310670
http://www.televic-rail.com
Televic Rail NV - Leo Bekaertlaan 1 - 8870 Izegem - Belgium
Company number 0825.539.581 - RPR Kortrijk