To LUGNET HomepageTo LUGNET News HomepageTo LUGNET Guide Homepage
 Help on Searching
 
Post new message to lugnet.robotics.rcx.legosOpen lugnet.robotics.rcx.legos in your NNTP NewsreaderTo LUGNET News Traffic PageSign In (Members)
 Robotics / RCX / legOS / 1644
1643  |  1645
Subject: 
Re: LegOS 0.2.5 and LNP
Newsgroups: 
lugnet.robotics.rcx.legos
Date: 
Sat, 20 Jan 2001 17:14:41 GMT
Viewed: 
1433 times
  
Bernardo Dal Seno <piu1759@cdc8g5.cdc.polimi.it> writes:

1. Now lnp_logical_write() is no more thread-safe.  Furthermore calling
   lnp_logical_write() screw up the semaphore (tx_sem) and so the other two
   lnp_XXX_write() become thread-unsafe.

2. On the host computer LNP is still thread unsafe.

I cleaned up my code (found one bug!), the patch follows.  As you may see
now there are two semaphore, because two are the resources to give access
to: the LNP buffer and the IR port.  I modified the calling of the handlers
to avoid race condition in a multithead envinronment on a PC.  My only
worry is the symbol the semaphore-related code in lnp.c should depend on.
I used the conditional

#if defined CONF_TM || defined CONF_HOST

Perhaps that breaks something in the utilities and we should introduce a
new symbol (CONF_HOST_TM?), I don't know for sure.  I leave this point to
more savvy people.

bye
Bernardo


Patch:

diff -Naur legOS-0.2.5/include/lnp/lnp.h legOS-0.2.5+/include/lnp/lnp.h
--- legOS-0.2.5/include/lnp/lnp.h Mon Aug 14 11:03:42 2000
+++ legOS-0.2.5+/include/lnp/lnp.h Wed Jan 17 23:00:16 2001
@@ -64,10 +64,10 @@

//! there are no ports for integrity layer packets, so there's just
//  one handler.
-extern lnp_integrity_handler_t lnp_integrity_handler;
+extern volatile lnp_integrity_handler_t lnp_integrity_handler;

//! addressing layer packets may be directed to a variety of ports.
-extern lnp_addressing_handler_t lnp_addressing_handler[];
+extern volatile lnp_addressing_handler_t lnp_addressing_handler[];

///////////////////////////////////////////////////////////////////////
//
diff -Naur legOS-0.2.5/kernel/lnp-logical.c legOS-0.2.5+/kernel/lnp-logical.c
--- legOS-0.2.5/kernel/lnp-logical.c Mon Aug 14 22:43:26 2000
+++ legOS-0.2.5+/kernel/lnp-logical.c Sat Jan 20 17:57:08 2001
@@ -66,7 +66,7 @@
static time_t allow_tx;             //!< time to allow new transmission

#ifdef CONF_TM
-sem_t tx_sem;             //!< transmitter access semaphore
+static sem_t tx_sem;             //!< transmitter access semaphore
#endif

///////////////////////////////////////////////////////////////////////////////
@@ -259,10 +259,10 @@
*/
int lnp_logical_write(const void* buf,size_t len) {
   unsigned char tmp;
-
-//#ifdef CONF_TM
-//  sem_wait(&tx_sem);
-//#endif
+
+#ifdef CONF_TM
+  sem_wait(&tx_sem);
+#endif

#ifdef CONF_AUTOSHUTOFF
   shutoff_restart();
diff -Naur legOS-0.2.5/kernel/lnp.c legOS-0.2.5+/kernel/lnp.c
--- legOS-0.2.5/kernel/lnp.c Mon Aug 14 22:43:26 2000
+++ legOS-0.2.5+/kernel/lnp.c Fri Jan 19 19:18:02 2001
@@ -54,16 +54,15 @@
//
///////////////////////////////////////////////////////////////////////////////

-#ifndef CONF_HOST
-#ifdef CONF_TM
+
+#if defined CONF_TM || defined CONF_HOST

#include <semaphore.h>

-//!< transmitter access semaphore
-extern sem_t tx_sem;
+//!< tansmit buffer access semaphore
+static sem_t buf_sem;

#endif
-#endif

//! the timeout counter in ms
volatile unsigned short lnp_timeout_counter;
@@ -77,13 +76,13 @@
//! there are no ports for integrity layer packets, so just one handler.
/*! FIXME: uninitialized
*/
-lnp_integrity_handler_t lnp_integrity_handler;
+volatile lnp_integrity_handler_t lnp_integrity_handler;

//! addressing layer packets may be directed to a variety of ports.
/*! FIXME: uninitialized
     FIXME: inefficient if LNP_PORT_MASK doesn't adhere to 0...01...1 scheme.
*/
-lnp_addressing_handler_t lnp_addressing_handler[LNP_PORTMASK+1];
+volatile lnp_addressing_handler_t lnp_addressing_handler[LNP_PORTMASK+1];

//! the LNP transmit buffer
static unsigned char lnp_buffer[256+3];
@@ -141,10 +140,10 @@
*/
int lnp_integrity_write(const unsigned char *data,unsigned char length) {

-#ifndef CONF_HOST
-#ifdef CONF_TM
-  sem_wait(&tx_sem);
-#endif
+  int r;
+
+#if defined CONF_TM || defined CONF_HOST
+  sem_wait(&buf_sem);
#endif

   lnp_buffer[0]=0xf0;
@@ -152,7 +151,12 @@
   memcpy(lnp_buffer+2,data,length);
   lnp_buffer[length+2]=(unsigned char) lnp_checksum(lnp_buffer,length+2);

-  return lnp_logical_write(lnp_buffer,length+3);
+  r = lnp_logical_write(lnp_buffer,length+3);
+
+#if defined CONF_TM || defined CONF_HOST
+  sem_post(&buf_sem);
+#endif
+  return r;
}

//! send a LNP addressing layer packet of given length
@@ -161,10 +165,10 @@
int lnp_addressing_write(const unsigned char *data,unsigned char length,
                          unsigned char dest,unsigned char srcport) {

-#ifndef CONF_HOST
-#ifdef CONF_TM
-  sem_wait(&tx_sem);
-#endif
+  int r;
+
+#if defined CONF_TM || defined CONF_HOST
+  sem_wait(&buf_sem);
#endif

   lnp_buffer[0]=0xf1;
@@ -174,23 +178,31 @@
   memcpy(lnp_buffer+4,data,length);
   lnp_buffer[length+4]=(unsigned char) lnp_checksum(lnp_buffer,length+4);

-  return lnp_logical_write(lnp_buffer,length+5);
+  r = lnp_logical_write(lnp_buffer,length+5);
+
+#if defined CONF_TM || defined CONF_HOST
+  sem_post(&buf_sem);
+#endif
+
+  return r;
}

//! handle LNP packet from the integrity layer
void lnp_receive_packet(const unsigned char *data) {
   unsigned char header=*(data++);
   unsigned char length=*(data++);
+  lnp_integrity_handler_t intgh;
+  lnp_addressing_handler_t addrh;

   // only handle non-degenerate packets in boot protocol 0xf0
   //
   switch(header) {
     case 0xf0:       // raw integrity layer packet, no addressing.
-      if(lnp_integrity_handler) {
+      if((intgh = lnp_integrity_handler)) {
#ifdef CONF_AUTOSHUTOFF
shutoff_restart();
#endif
-        lnp_integrity_handler(data,length);
+        intgh(data,length);
       }
       break;

@@ -201,12 +213,12 @@
if(LNP_HOSTADDR == (dest & LNP_HOSTMASK)) {
  unsigned char port=dest & LNP_PORTMASK;

-   if(lnp_addressing_handler[port]) {
+   if((addrh = lnp_addressing_handler[port])) {
        unsigned char src=*(data++);
#ifdef CONF_AUTOSHUTOFF
    shutoff_restart();
#endif
-     lnp_addressing_handler[port](data,length-2,src);
+     addrh(data,length-2,src);
  }
}
       }
@@ -320,6 +332,10 @@
   for(k=1;k<=LNP_PORTMASK;k++)
     lnp_addressing_handler[k]=LNP_DUMMY_ADDRESSING;
   lnp_integrity_handler=LNP_DUMMY_INTEGRITY;
+
+#if defined CONF_TM || defined CONF_HOST
+  sem_init(&buf_sem,0,1);
+#endif
}

#endif // CONF_LNP



Message has 1 Reply:
  Re: LegOS 0.2.5 and LNP
 
(...) you may see (...) give access (...) the handlers (...) My only (...) depend on. (...) introduce a (...) this point to (...) I think that we can use safely (CONF_TM || defined CONF_HOST) condition. I'm checking the whole patch and I think that (...) (24 years ago, 21-Jan-01, to lugnet.robotics.rcx.legos)

Message is in Reply To:
  LegOS 0.2.5 and LNP
 
Yesterday I downloaded legOS 0.2.5 and peeked at the code. Btw. README is not up to date. For last two month I was working with a friend of mine on a transport protocol implemented on top of LNP for the transmission of packets of unlimited size and (...) (24 years ago, 17-Jan-01, to lugnet.robotics.rcx.legos)

9 Messages in This Thread:


Entire Thread on One Page:
Nested:  All | Brief | Compact | Dots
Linear:  All | Brief | Compact

This Message and its Replies on One Page:
Nested:  All | Brief | Compact | Dots
Linear:  All | Brief | Compact
    

Custom Search

©2005 LUGNET. All rights reserved. - hosted by steinbruch.info GbR