Re: RFC: bsd_tcpip patch on synch.c

[Lambrecht Jürgen <J dot Lambrecht at TELEVIC dot com>]

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