* RFC: bsd_tcpip patch on synch.c
@ 2013-06-27 9:34 Lambrecht Jürgen
[not found] ` <DUB124-W44EE5E989F221552E035FDE4750@phx.gbl>
0 siblings, 1 reply; 4+ messages in thread
From: Lambrecht Jürgen @ 2013-06-27 9:34 UTC (permalink / raw)
To: ecos-devel; +Cc: Bernd Edlinger
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: RFC: bsd_tcpip patch on synch.c
[not found] ` <DUB124-W44EE5E989F221552E035FDE4750@phx.gbl>
@ 2013-06-27 12:51 ` Bernd Edlinger
2013-06-27 14:20 ` Lambrecht Jürgen
0 siblings, 1 reply; 4+ messages in thread
From: Bernd Edlinger @ 2013-06-27 12:51 UTC (permalink / raw)
To: ecos-devel
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.
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.
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: RFC: bsd_tcpip patch on synch.c
2013-06-27 12:51 ` Bernd Edlinger
@ 2013-06-27 14:20 ` Lambrecht Jürgen
[not found] ` <DUB124-W17E1B3022EDF1B00BF4396E4750@phx.gbl>
0 siblings, 1 reply; 4+ messages in thread
From: Lambrecht Jürgen @ 2013-06-27 14:20 UTC (permalink / raw)
To: Bernd Edlinger; +Cc: ecos-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* FW: RFC: bsd_tcpip patch on synch.c
[not found] ` <DUB124-W17E1B3022EDF1B00BF4396E4750@phx.gbl>
@ 2013-06-27 17:07 ` Bernd Edlinger
0 siblings, 0 replies; 4+ messages in thread
From: Bernd Edlinger @ 2013-06-27 17:07 UTC (permalink / raw)
To: j.lambrecht; +Cc: ecos-devel
Jürgen,
> 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?
no, IMHO a simple "cyg_mutex_lock(&spl_mutex);" would have been enough.
But these patches are not about style at all.
But by the way there is another patch in that vicinitry:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001629
That may apply to everyone who uses a tick count other than the default.
Regards
Bernd.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-06-27 17:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-27 9:34 RFC: bsd_tcpip patch on synch.c Lambrecht Jürgen
[not found] ` <DUB124-W44EE5E989F221552E035FDE4750@phx.gbl>
2013-06-27 12:51 ` Bernd Edlinger
2013-06-27 14:20 ` Lambrecht Jürgen
[not found] ` <DUB124-W17E1B3022EDF1B00BF4396E4750@phx.gbl>
2013-06-27 17:07 ` FW: " Bernd Edlinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).