TaiJuWu commented on code in PR #10776:
URL: https://github.com/apache/nuttx/pull/10776#discussion_r1354294685


##########
sched/irq/irq_spinlock.c:
##########
@@ -209,4 +220,202 @@ void spin_unlock_irqrestore_wo_note(spinlock_t *lock, 
irqstate_t flags)
   up_irq_restore(flags);
 }
 
+#ifdef CONFIG_RW_SPINLOCK
+
+/****************************************************************************
+ * Name: read_lock_irqsave
+ *
+ * Description:
+ *   If SMP is are enabled:
+ *     If the argument lock is not specified (i.e. NULL), disable local
+ *     interrupts and take the global read write spinlock (g_irq_rw_spin)
+ *     and increase g_irq_rw_spin.
+ *
+ *     If the argument lock is specified,
+ *     disable local interrupts and take the lock spinlock and return
+ *     the interrupt state.
+ *
+ *     NOTE: This API is very simple to protect data (e.g. H/W register
+ *     or internal data structure) in SMP mode. But do not use this API
+ *     with kernel APIs which suspend a caller thread. (e.g. nxsem_wait)
+ *
+ *   If SMP is not enabled:
+ *     This function is equivalent to up_irq_save().
+ *
+ * Input Parameters:
+ *   lock - Caller specific spinlock. If specified NULL, g_irq_spin is used
+ *          and can be nested. Otherwise, nested call for the same lock
+ *          would cause a deadlock
+ *
+ * Returned Value:
+ *   An opaque, architecture-specific value that represents the state of
+ *   the interrupts prior to the call to write_lock_irqsave(lock);
+ *
+ ****************************************************************************/
+
+irqstate_t read_lock_irqsave(FAR rwlock_t *lock)
+{
+  irqstate_t ret;
+  ret = up_irq_save();
+
+  if (NULL == lock)
+    {
+      read_lock(&g_irq_rw_spin);
+    }
+  else
+    {
+      read_lock(lock);
+    }
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: read_unlock_irqrestore
+ *
+ * Description:
+ *   If SMP is enabled:
+ *     If the argument lock is not specified (i.e. NULL),
+ *     decrement the call counter (g_irq_rw_spin) and restore the interrupt
+ *     state as it was prior to the previous call to read_lock_irqsave(NULL).
+ *
+ *     If the argument lock is specified, release the lock and
+ *     restore the interrupt state as it was prior to the previous call to
+ *     read_lock_irqsave(lock).
+ *
+ *   If SMP is not enabled:
+ *     This function is equivalent to up_irq_restore().
+ *
+ * Input Parameters:
+ *   lock - Caller specific spinlock. If specified NULL, g_irq_spin is used.
+ *
+ *   flags - The architecture-specific value that represents the state of
+ *           the interrupts prior to the call to read_lock_irqsave(lock);
+ *
+ * Returned Value:
+ *   None
+ *
+ ****************************************************************************/
+
+void read_unlock_irqrestore(rwlock_t *lock, irqstate_t flags)
+{
+  if (NULL == lock)
+    {
+      read_unlock(&g_irq_rw_spin);
+    }
+  else
+    {
+      read_unlock(lock);
+    }
+
+  up_irq_restore(flags);
+}
+
+/****************************************************************************
+ * Name: write_lock_irqsave
+ *
+ * Description:
+ *   If SMP is are enabled:
+ *     If the argument lock is not specified (i.e. NULL),
+ *     disable local interrupts and take the global spinlock (g_irq_rw_spin)
+ *     if the call counter (g_irq_write_spin_count[cpu]) equals to 0. Then
+ *     the counter on the CPU is incremented to allow nested calls and return
+ *     the interrupt state.
+ *
+ *     If the argument lock is specified,
+ *     disable local interrupts and take the lock spinlock and return
+ *     the interrupt state.
+ *
+ *     NOTE: This API is very simple to protect data (e.g. H/W register
+ *     or internal data structure) in SMP mode. But do not use this API
+ *     with kernel APIs which suspend a caller thread. (e.g. nxsem_wait)
+ *
+ *   If SMP is not enabled:
+ *     This function is equivalent to up_irq_save().
+ *
+ * Input Parameters:
+ *   lock - Caller specific spinlock. If specified NULL, g_irq_spin is used
+ *          and can be nested. Otherwise, nested call for the same lock
+ *          would cause a deadlock
+ *
+ * Returned Value:
+ *   An opaque, architecture-specific value that represents the state of
+ *   the interrupts prior to the call to write_lock_irqsave(lock);
+ *
+ ****************************************************************************/
+
+irqstate_t write_lock_irqsave(rwlock_t *lock)
+{
+  irqstate_t ret;
+  ret = up_irq_save();
+
+  if (NULL == lock)
+    {
+      int me = this_cpu();
+      if (0 == g_irq_write_spin_count[me])

Review Comment:
   Align g_irq_spin_count.
   If the lock is NULL, the caller will get global lock (e.g. g_irq_rw_spin) 
and spin_lock_irqsave() support nest on the same CPU.
   
   If the CPU can write lock, it can call write_lock_irqsave() again (e.g. 
support nest).
   
   Why is the variable called g_irq_write_spin_count?
   Because reader can read data simultaneously and the counter is already 
record in lock.
   Based on the reason, we only need to record the counter of the cpu get the 
write lock (nest).
   
   But g_irq_rw_spin_count is still fine.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to