This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git

commit 02e7ed7cc4be42606db8618107c6c55558d7413e
Author: zhanghongyu <[email protected]>
AuthorDate: Thu Jun 19 14:03:24 2025 +0800

    net/can: replace net_lock with conn_lock and conn_lock_dev
    
    Protect can resources through netdev_lock, conn_lock, and can_list_lock
    
    Signed-off-by: zhanghongyu <[email protected]>
---
 net/can/can.h                  | 16 ++++++++++++++++
 net/can/can_callback.c         | 14 +++++++-------
 net/can/can_conn.c             | 26 ++++++++++++++++++++++++++
 net/can/can_input.c            | 10 +++++++++-
 net/can/can_recvmsg.c          | 12 +++++++++---
 net/can/can_sendmsg.c          |  9 +++++++--
 net/can/can_sendmsg_buffered.c | 18 ++++++------------
 net/can/can_setsockopt.c       |  4 ++--
 net/can/can_sockif.c           |  8 ++++++--
 net/devif/devif_poll.c         |  2 ++
 10 files changed, 90 insertions(+), 29 deletions(-)

diff --git a/net/can/can.h b/net/can/can.h
index fa7bf105384..25d252fbd09 100644
--- a/net/can/can.h
+++ b/net/can/can.h
@@ -188,6 +188,22 @@ void can_free(FAR struct can_conn_s *conn);
 
 FAR struct can_conn_s *can_nextconn(FAR struct can_conn_s *conn);
 
+/****************************************************************************
+ * Name: can_conn_list_lock
+ *       can_conn_list_unlock
+ *
+ * Description:
+ *   Lock and unlock the CAN connection list. This is used to protect
+ *   the list of active connections.
+ *
+ * Assumptions:
+ *   This function is called from driver.
+ *
+ ****************************************************************************/
+
+void can_conn_list_lock(void);
+void can_conn_list_unlock(void);
+
 /****************************************************************************
  * Name: can_active()
  *
diff --git a/net/can/can_callback.c b/net/can/can_callback.c
index 62538865151..e022f8ebd27 100644
--- a/net/can/can_callback.c
+++ b/net/can/can_callback.c
@@ -37,6 +37,7 @@
 #include <nuttx/net/can.h>
 
 #include "devif/devif.h"
+#include "utils/utils.h"
 #include "can/can.h"
 
 #ifdef CONFIG_NET_TIMESTAMP
@@ -149,13 +150,9 @@ uint16_t can_callback(FAR struct net_driver_s *dev,
             }
 #endif
 
-      /* Try to lock the network when successful send data to the listener */
-
-      if (net_trylock() == OK)
-        {
-          flags = devif_conn_event(dev, flags, conn->sconn.list);
-          net_unlock();
-        }
+      conn_lock(&conn->sconn);
+      flags = devif_conn_event(dev, flags, conn->sconn.list);
+      conn_unlock(&conn->sconn);
 
       /* Either we did not get the lock or there is no application listening
        * If we did not get a lock we store the frame in the read-ahead buffer
@@ -202,6 +199,7 @@ uint16_t can_datahandler(FAR struct net_driver_s *dev,
   FAR struct iob_s *iob = dev->d_iob;
   int ret = 0;
 
+  conn_lock(&conn->sconn);
 #if CONFIG_NET_RECV_BUFSIZE > 0
 #  if CONFIG_NET_CAN_NBUFFERS > 0
   int bufnum = div_const_roundup(conn->rcvbufs, NET_CAN_PKTSIZE);
@@ -244,10 +242,12 @@ uint16_t can_datahandler(FAR struct net_driver_s *dev,
       goto errout;
     }
 
+  conn_unlock(&conn->sconn);
   return ret;
 
 errout:
   netdev_iob_release(dev);
+  conn_unlock(&conn->sconn);
   return ret;
 }
 
diff --git a/net/can/can_conn.c b/net/can/can_conn.c
index 8daa2e2b740..69f41b813b0 100644
--- a/net/can/can_conn.c
+++ b/net/can/can_conn.c
@@ -138,6 +138,7 @@ void can_free(FAR struct can_conn_s *conn)
   /* Remove the connection from the active list */
 
   dq_rem(&conn->sconn.node, &g_active_can_connections);
+  nxmutex_destroy(&conn->sconn.s_lock);
 
 #ifdef CONFIG_NET_CAN_WRITE_BUFFERS
   /* Free the write queue */
@@ -243,6 +244,29 @@ FAR struct can_conn_s *can_nextconn(FAR struct can_conn_s 
*conn)
     }
 }
 
+/****************************************************************************
+ * Name: can_conn_list_lock
+ *       can_conn_list_unlock
+ *
+ * Description:
+ *   Lock and unlock the CAN connection list. This is used to protect
+ *   the list of active connections.
+ *
+ * Assumptions:
+ *   This function is called from driver.
+ *
+ ****************************************************************************/
+
+void can_conn_list_lock(void)
+{
+  NET_BUFPOOL_LOCK(g_can_connections);
+}
+
+void can_conn_list_unlock(void)
+{
+  NET_BUFPOOL_UNLOCK(g_can_connections);
+}
+
 /****************************************************************************
  * Name: can_active()
  *
@@ -267,6 +291,7 @@ FAR struct can_conn_s *can_active(FAR struct net_driver_s 
*dev,
   memcpy(&can_id, NETLLBUF, sizeof(canid_t));
 #endif
 
+  can_conn_list_lock();
   while ((conn = can_nextconn(conn)) != NULL)
     {
       if ((conn->dev == NULL && _SS_ISBOUND(conn->sconn.s_flags)) ||
@@ -282,6 +307,7 @@ FAR struct can_conn_s *can_active(FAR struct net_driver_s 
*dev,
         }
     }
 
+  can_conn_list_unlock();
   return conn;
 }
 
diff --git a/net/can/can_input.c b/net/can/can_input.c
index 7a30fc3febd..d73a958ccc1 100644
--- a/net/can/can_input.c
+++ b/net/can/can_input.c
@@ -35,6 +35,7 @@
 #include <nuttx/net/netstats.h>
 
 #include "devif/devif.h"
+#include "utils/utils.h"
 #include "can/can.h"
 
 /****************************************************************************
@@ -218,6 +219,7 @@ static int can_in(FAR struct net_driver_s *dev)
 {
   FAR struct can_conn_s *conn = can_active(dev, NULL);
   FAR struct can_conn_s *nextconn;
+  int ret;
 
   if (conn == NULL)
     {
@@ -226,6 +228,8 @@ static int can_in(FAR struct net_driver_s *dev)
       return OK;
     }
 
+  can_conn_list_lock();
+
   /* Do we have second connection that can hold this packet? */
 
   while ((nextconn = can_active(dev, conn)) != NULL)
@@ -250,7 +254,11 @@ static int can_in(FAR struct net_driver_s *dev)
 
   /* We can deliver the packet directly to the last listener. */
 
-  return can_input_conn(dev, conn);
+  ret = can_input_conn(dev, conn);
+
+  can_conn_list_unlock();
+
+  return ret;
 }
 
 /****************************************************************************
diff --git a/net/can/can_recvmsg.c b/net/can/can_recvmsg.c
index a0965e80f08..38440493ef4 100644
--- a/net/can/can_recvmsg.c
+++ b/net/can/can_recvmsg.c
@@ -47,6 +47,7 @@
 #include "devif/devif.h"
 #include "can/can.h"
 #include "socket/socket.h"
+#include "utils/utils.h"
 #include <netpacket/packet.h>
 
 #ifdef CONFIG_NET_TIMESTAMP
@@ -419,7 +420,7 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
                     int flags)
 {
   FAR struct can_conn_s *conn;
-  FAR struct net_driver_s *dev;
+  FAR struct net_driver_s *dev = NULL;
   struct can_recvfrom_s state;
   int ret;
 
@@ -436,7 +437,7 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
       return -ENOTSUP;
     }
 
-  net_lock();
+  conn_lock(&conn->sconn);
 
   /* Initialize the state structure. */
 
@@ -504,6 +505,9 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
       goto errout_with_state;
     }
 
+  conn_unlock(&conn->sconn);
+  conn_dev_lock(&conn->sconn, dev);
+
   /* Set up the callback in the connection */
 
   state.pr_cb = can_callback_alloc(dev, conn);
@@ -519,7 +523,9 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
        * the task sleeps and automatically re-locked when the task restarts.
        */
 
+      conn_dev_unlock(&conn->sconn, dev);
       ret = net_sem_wait(&state.pr_sem);
+      conn_dev_lock(&conn->sconn, dev);
 
       /* Make sure that no further events are processed */
 
@@ -532,7 +538,7 @@ ssize_t can_recvmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
     }
 
 errout_with_state:
-  net_unlock();
+  conn_dev_unlock(&conn->sconn, dev);
   nxsem_destroy(&state.pr_sem);
   return ret;
 }
diff --git a/net/can/can_sendmsg.c b/net/can/can_sendmsg.c
index c85242dc892..120eb7e8ada 100644
--- a/net/can/can_sendmsg.c
+++ b/net/can/can_sendmsg.c
@@ -47,6 +47,7 @@
 #include "netdev/netdev.h"
 #include "devif/devif.h"
 #include "socket/socket.h"
+#include "utils/utils.h"
 #include "can/can.h"
 
 #include <sys/time.h>
@@ -225,7 +226,7 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
    * because we don't want anything to happen until we are ready.
    */
 
-  net_lock();
+  conn_dev_lock(&conn->sconn, dev);
   memset(&state, 0, sizeof(struct send_s));
   nxsem_init(&state.snd_sem, 0, 0); /* Doesn't really fail */
 
@@ -258,6 +259,8 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
       state.snd_cb->priv  = (FAR void *)&state;
       state.snd_cb->event = psock_send_eventhandler;
 
+      conn_dev_unlock(&conn->sconn, dev);
+
       /* Notify the device driver that new TX data is available. */
 
       netdev_txnotify_dev(dev);
@@ -275,13 +278,15 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
           ret = net_sem_timedwait(&state.snd_sem, UINT_MAX);
         }
 
+      conn_dev_lock(&conn->sconn, dev);
+
       /* Make sure that no further events are processed */
 
       can_callback_free(dev, conn, state.snd_cb);
     }
 
   nxsem_destroy(&state.snd_sem);
-  net_unlock();
+  conn_dev_unlock(&conn->sconn, dev);
 
   /* Check for a errors, Errors are signalled by negative errno values
    * for the send length
diff --git a/net/can/can_sendmsg_buffered.c b/net/can/can_sendmsg_buffered.c
index 22078182646..523fcaab54c 100644
--- a/net/can/can_sendmsg_buffered.c
+++ b/net/can/can_sendmsg_buffered.c
@@ -248,7 +248,7 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
    * because we don't want anything to happen until we are ready.
    */
 
-  net_lock();
+  conn_dev_lock(&conn->sconn, dev);
 
 #if CONFIG_NET_SEND_BUFSIZE > 0
   if ((iob_get_queue_size(&conn->write_q) + msg->msg_iov->iov_len) >
@@ -313,21 +313,15 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
     }
   else
     {
-      unsigned int count;
-      int blresult;
-
       /* iob_copyin might wait for buffers to be freed, but if
        * network is locked this might never happen, since network
        * driver is also locked, therefore we need to break the lock
        */
 
-      blresult = net_breaklock(&count);
+      conn_dev_unlock(&conn->sconn, dev);
       ret = iob_copyin(wb_iob, (FAR uint8_t *)msg->msg_iov->iov_base,
                        msg->msg_iov->iov_len, 0, false);
-      if (blresult >= 0)
-        {
-          net_restorelock(count);
-        }
+      conn_dev_lock(&conn->sconn, dev);
     }
 
   if (ret < 0)
@@ -389,7 +383,7 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
 
       /* unlock */
 
-      net_unlock();
+      conn_dev_unlock(&conn->sconn, dev);
 
       /* Notify the device driver that new TX data is available. */
 
@@ -399,7 +393,7 @@ ssize_t can_sendmsg(FAR struct socket *psock, FAR struct 
msghdr *msg,
     {
       /* unlock */
 
-      net_unlock();
+      conn_dev_unlock(&conn->sconn, dev);
     }
 
   return msg->msg_iov->iov_len;
@@ -411,7 +405,7 @@ errout_with_wq:
   iob_free_queue(&conn->write_q);
 
 errout_with_lock:
-  net_unlock();
+  conn_dev_unlock(&conn->sconn, dev);
   return ret;
 }
 
diff --git a/net/can/can_setsockopt.c b/net/can/can_setsockopt.c
index 692df5ad0c6..b6f4066dba9 100644
--- a/net/can/can_setsockopt.c
+++ b/net/can/can_setsockopt.c
@@ -158,7 +158,7 @@ int can_setsockopt(FAR struct socket *psock, int level, int 
option,
          * options.
          */
 
-        net_lock();
+        conn_lock(&conn->sconn);
 
         /* Set or clear the option bit */
 
@@ -171,7 +171,7 @@ int can_setsockopt(FAR struct socket *psock, int level, int 
option,
             _SO_CLROPT(conn->sconn.s_options, option);
           }
 
-        net_unlock();
+        conn_unlock(&conn->sconn);
         break;
 
 #if CONFIG_NET_RECV_BUFSIZE > 0
diff --git a/net/can/can_sockif.c b/net/can/can_sockif.c
index 1a3e280cab3..5084363f0f8 100644
--- a/net/can/can_sockif.c
+++ b/net/can/can_sockif.c
@@ -43,6 +43,7 @@
 
 #include "can/can.h"
 #include "netdev/netdev.h"
+#include "utils/utils.h"
 
 #ifdef CONFIG_NET_CAN
 
@@ -235,6 +236,7 @@ static int can_setup(FAR struct socket *psock)
       conn->sndbufs = CONFIG_NET_SEND_BUFSIZE;
       nxsem_init(&conn->sndsem, 0, 0);
 #endif
+      nxmutex_init(&conn->sconn.s_lock);
 
       /* Attach the connection instance to the socket */
 
@@ -388,7 +390,7 @@ static int can_poll_local(FAR struct socket *psock, FAR 
struct pollfd *fds,
 
   if (setup)
     {
-      net_lock();
+      conn_dev_lock(&conn->sconn, conn->dev);
 
       info->dev = conn->dev;
 
@@ -451,7 +453,7 @@ static int can_poll_local(FAR struct socket *psock, FAR 
struct pollfd *fds,
       poll_notify(&fds, 1, eventset);
 
 errout_with_lock:
-      net_unlock();
+      conn_dev_unlock(&conn->sconn, conn->dev);
     }
   else
     {
@@ -461,7 +463,9 @@ errout_with_lock:
         {
           /* Cancel any response notifications */
 
+          conn_dev_lock(&conn->sconn, info->dev);
           can_callback_free(info->dev, conn, info->cb);
+          conn_dev_unlock(&conn->sconn, info->dev);
 
           /* Release the poll/select data slot */
 
diff --git a/net/devif/devif_poll.c b/net/devif/devif_poll.c
index 02b05131de9..64a335c1f3c 100644
--- a/net/devif/devif_poll.c
+++ b/net/devif/devif_poll.c
@@ -288,6 +288,7 @@ static int devif_poll_can_connections(FAR struct 
net_driver_s *dev,
    * perform the poll action
    */
 
+  can_conn_list_lock();
   while (!bstop && (can_conn = can_nextconn(can_conn)))
     {
       /* Skip connections that are bound to other polling devices */
@@ -307,6 +308,7 @@ static int devif_poll_can_connections(FAR struct 
net_driver_s *dev,
         }
     }
 
+  can_conn_list_unlock();
   return bstop;
 }
 #endif /* CONFIG_NET_PKT */

Reply via email to