mm/iob: fix some comment in #14764

reason: Since we decoupled counting and sem count, we changed the meanings of three key global variables: g_iob_count: A positive number indicates the available number of IOBs, while a negative number indicates the number of waiters in iob_alloc (when throttle == false). g_throttle_wait: Represents the number of waiters in iob_alloc (when throttle == true), and it will not be negative. g_qentry_wait: Represents the number of waiters for qentry, and it will not be negative. Signed-off-by: hujun5 <[email protected]>
apache · Nov 19, 2024 · 9f835f6 · 9f835f6
1 parent f142d04
commit 9f835f6
Show file tree

Hide file tree

Showing 9 changed files with 45 additions and 161 deletions.
diff --git a/include/nuttx/mm/iob.h b/include/nuttx/mm/iob.h
@@ -278,16 +278,6 @@ FAR struct iob_s *iob_alloc_with_data(FAR void *data, uint16_t size,
 
 int iob_navail(bool throttled);
 
-/****************************************************************************
- * Name: iob_qentry_navail
- *
- * Description:
- *   Return the number of available IOB chains.
- *
- ****************************************************************************/
-
-int iob_qentry_navail(void);
-
 /****************************************************************************
  * Name: iob_free
  *

diff --git a/mm/iob/iob.h b/mm/iob/iob.h
@@ -75,27 +75,27 @@ extern FAR struct iob_qentry_s *g_iob_freeqlist;
 extern FAR struct iob_qentry_s *g_iob_qcommitted;
 #endif
 
-/* Counting semaphores that tracks the number of free IOBs/qentries */
+/* semaphores that IOBs need wait */
 
 extern sem_t g_iob_sem;
 
 /* Counts free I/O buffers */
 
-extern volatile int16_t g_iob_count;
+extern int16_t g_iob_count;
 
 #if CONFIG_IOB_THROTTLE > 0
 extern sem_t g_throttle_sem;
 
-/* Counts available I/O buffers when throttled */
+/* Wait Counts for throttle */
 
-extern volatile int16_t g_throttle_count;
+extern int16_t g_throttle_wait;
 #endif
 #if CONFIG_IOB_NCHAINS > 0
 extern sem_t g_qentry_sem;
 
-/* Counts free I/O buffer queue containers */
+/* Wait Counts for qentry */
 
-extern volatile int16_t g_qentry_count;
+extern int16_t g_qentry_wait;
 #endif
 
 extern volatile spinlock_t g_iob_lock;

diff --git a/mm/iob/iob_alloc.c b/mm/iob/iob_alloc.c
@@ -105,20 +105,9 @@ static FAR struct iob_s *iob_tryalloc_internal(bool throttled)
 {
   FAR struct iob_s *iob;
 #if CONFIG_IOB_THROTTLE > 0
-  int16_t count;
-#endif
-
-#if CONFIG_IOB_THROTTLE > 0
-  /* Select the count to check. */
-
-  count = (throttled ? g_throttle_count : g_iob_count);
-#endif
-
-  /* We don't know what context we are called from so we use extreme measures
-   * to protect the free list:  We disable interrupts very briefly.
-   */
+  int16_t count = (throttled ? g_iob_count - CONFIG_IOB_THROTTLE :
+                   g_iob_count);
 
-#if CONFIG_IOB_THROTTLE > 0
   /* If there are free I/O buffers for this allocation */
 
   if (count > 0)
@@ -136,30 +125,9 @@ static FAR struct iob_s *iob_tryalloc_internal(bool throttled)
 
           g_iob_freelist = iob->io_flink;
 
-          /* Take a semaphore count.  Note that we cannot do this in
-           * in the orthodox way by calling nxsem_wait() or nxsem_trywait()
-           * because this function may be called from an interrupt
-           * handler. Fortunately we know at at least one free buffer
-           * so a simple decrement is all that is needed.
-           */
-
           g_iob_count--;
           DEBUGASSERT(g_iob_count >= 0);
 
-#if CONFIG_IOB_THROTTLE > 0
-          /* The throttle semaphore is used to throttle the number of
-           * free buffers that are available.  It is used to prevent
-           * the overrunning of the free buffer list. Please note that
-           * it can only be decremented to zero, which indicates no
-           * throttled buffers are available.
-           */
-
-          if (g_throttle_count > 0)
-            {
-              g_throttle_count--;
-            }
-#endif
-
           /* Put the I/O buffer in a known state */
 
           iob->io_flink  = NULL; /* Not in a chain */
@@ -185,19 +153,16 @@ static FAR struct iob_s *iob_tryalloc_internal(bool throttled)
 static FAR struct iob_s *iob_allocwait(bool throttled, unsigned int timeout)
 {
   FAR struct iob_s *iob;
-  FAR volatile int16_t *count;
   irqstate_t flags;
   FAR sem_t *sem;
   clock_t start;
   int ret = OK;
 
 #if CONFIG_IOB_THROTTLE > 0
-  /* Select the semaphore count to check. */
+  /* Select the semaphore to wait. */
 
-  count = (throttled ? &g_throttle_count : &g_iob_count);
   sem = (throttled ? &g_throttle_sem : &g_iob_sem);
 #else
-  count = &g_iob_count;
   sem = &g_iob_sem;
 #endif
 
@@ -209,20 +174,21 @@ static FAR struct iob_s *iob_allocwait(bool throttled, unsigned int timeout)
 
   flags = spin_lock_irqsave(&g_iob_lock);
 
-  /* Try to get an I/O buffer.  If successful, the semaphore count will be
-   * decremented atomically.
-   */
+  /* Try to get an I/O buffer */
 
-  iob   = iob_tryalloc_internal(throttled);
+  iob = iob_tryalloc_internal(throttled);
   if (iob == NULL)
     {
-      /* If not successful, then the semaphore count was less than or equal
-       * to zero (meaning that there are no free buffers).  We need to wait
-       * for an I/O buffer to be released and placed in the committed
-       * list.
-       */
-
-      (*count)--;
+#if CONFIG_IOB_THROTTLE > 0
+      if (throttled)
+        {
+          g_throttle_wait++;
+        }
+      else
+#endif
+        {
+          g_iob_count--;
+        }
 
       spin_unlock_irqrestore(&g_iob_lock, flags);
 

diff --git a/mm/iob/iob_alloc_qentry.c b/mm/iob/iob_alloc_qentry.c
@@ -97,16 +97,6 @@ static FAR struct iob_qentry_s *iob_tryalloc_qentry_internal(void)
 
       g_iob_freeqlist = iobq->qe_flink;
 
-      /* Take a semaphore count.  Note that we cannot do this in
-       * in the orthodox way by calling nxsem_wait() or nxsem_trywait()
-       * because this function may be called from an interrupt
-       * handler. Fortunately we know at at least one free buffer
-       * so a simple decrement is all that is needed.
-       */
-
-      g_qentry_count--;
-      DEBUGASSERT(g_qentry_count >= 0);
-
       /* Put the I/O buffer in a known state */
 
       iobq->qe_head = NULL; /* Nothing is contained */
@@ -139,20 +129,16 @@ static FAR struct iob_qentry_s *iob_allocwait_qentry(void)
 
   flags = spin_lock_irqsave(&g_iob_lock);
 
-  /* Try to get an I/O buffer chain container.  If successful, the semaphore
-   * count will bedecremented atomically.
-   */
+  /* Try to get an I/O buffer chain container. */
 
   qentry = iob_tryalloc_qentry_internal();
   if (qentry == NULL)
     {
-      /* If not successful, then the semaphore count was less than or equal
-       * to zero (meaning that there are no free buffers).  We need to wait
-       * for an I/O buffer chain container to be released when the
-       * semaphore count will be incremented.
+      /* If not successful, We need to wait
+       * for an I/O buffer chain container to be released
        */
 
-      g_qentry_count--;
+      g_qentry_wait++;
       spin_unlock_irqrestore(&g_iob_lock, flags);
       ret = nxsem_wait_uninterruptible(&g_qentry_sem);
       if (ret >= 0)

diff --git a/mm/iob/iob_free.c b/mm/iob/iob_free.c
@@ -141,59 +141,34 @@ FAR struct iob_s *iob_free(FAR struct iob_s *iob)
    * cases.
    */
 
-#if CONFIG_IOB_THROTTLE > 0
-  if ((g_iob_count < 0) ||
-      ((g_iob_count >= CONFIG_IOB_THROTTLE) &&
-       (g_throttle_count < 0)))
-#else
   if (g_iob_count < 0)
-#endif
     {
-      FAR sem_t *sem;
-
+      g_iob_count++;
       iob->io_flink   = g_iob_committed;
       g_iob_committed = iob;
-
+      spin_unlock_irqrestore(&g_iob_lock, flags);
+      nxsem_post(&g_iob_sem);
+    }
 #if CONFIG_IOB_THROTTLE > 0
-      if (g_iob_count < 0)
-        {
-          g_iob_count++;
-          sem = &g_iob_sem;
-        }
-      else
-        {
-          g_throttle_count++;
-          sem = &g_throttle_sem;
-        }
-#else
-      g_iob_count++;
-      sem = &g_iob_sem;
-#endif
+  else if (g_throttle_wait > 0 && g_iob_count >= CONFIG_IOB_THROTTLE)
+    {
+      iob->io_flink   = g_iob_committed;
+      g_iob_committed = iob;
+      g_throttle_wait--;
       spin_unlock_irqrestore(&g_iob_lock, flags);
-      nxsem_post(sem);
+      nxsem_post(&g_throttle_sem);
     }
+#endif
   else
     {
       g_iob_count++;
-#if CONFIG_IOB_THROTTLE > 0
-      if (g_iob_count > CONFIG_IOB_THROTTLE)
-        {
-          g_throttle_count++;
-        }
-#endif
-
       iob->io_flink   = g_iob_freelist;
       g_iob_freelist  = iob;
       spin_unlock_irqrestore(&g_iob_lock, flags);
     }
 
   DEBUGASSERT(g_iob_count <= CONFIG_IOB_NBUFFERS);
 
-#if CONFIG_IOB_THROTTLE > 0
-  DEBUGASSERT(g_throttle_count <=
-              (CONFIG_IOB_NBUFFERS - CONFIG_IOB_THROTTLE));
-#endif
-
 #ifdef CONFIG_IOB_NOTIFIER
   /* Check if the IOB was claimed by a thread that is blocked waiting
    * for an IOB.

diff --git a/mm/iob/iob_free_qentry.c b/mm/iob/iob_free_qentry.c
@@ -68,17 +68,16 @@ FAR struct iob_qentry_s *iob_free_qentry(FAR struct iob_qentry_s *iobq)
    * iob_tryalloc_qentry()).
    */
 
-  if (g_qentry_count < 0)
+  if (g_qentry_wait > 0)
     {
       iobq->qe_flink   = g_iob_qcommitted;
       g_iob_qcommitted = iobq;
-      g_qentry_count++;
+      g_qentry_wait--;
       spin_unlock_irqrestore(&g_iob_lock, flags);
       nxsem_post(&g_qentry_sem);
     }
   else
     {
-      g_qentry_count++;
       iobq->qe_flink   = g_iob_freeqlist;
       g_iob_freeqlist  = iobq;
       spin_unlock_irqrestore(&g_iob_lock, flags);

diff --git a/mm/iob/iob_initialize.c b/mm/iob/iob_initialize.c
@@ -93,26 +93,25 @@ FAR struct iob_qentry_s *g_iob_qcommitted;
 
 sem_t g_iob_sem = SEM_INITIALIZER(0);
 
-/* Counting semaphores that tracks the number of free IOBs/qentries */
+/* Counting that tracks the number of free IOBs/qentries */
 
-volatile int16_t g_iob_count = CONFIG_IOB_NBUFFERS;
+int16_t g_iob_count = CONFIG_IOB_NBUFFERS;
 
 #if CONFIG_IOB_THROTTLE > 0
 
 sem_t g_throttle_sem = SEM_INITIALIZER(0);
 
-/* Counts available I/O buffers when throttled */
+/* Wait Counts for throttle */
 
-volatile int16_t g_throttle_count = CONFIG_IOB_NBUFFERS -
-                                    CONFIG_IOB_THROTTLE;
+int16_t g_throttle_wait = 0;
 #endif
 
 #if CONFIG_IOB_NCHAINS > 0
 sem_t g_qentry_sem = SEM_INITIALIZER(0);
 
-/* Counts free I/O buffer queue containers */
+/* Wait Counts for qentry */
 
-volatile int16_t g_qentry_count = CONFIG_IOB_NCHAINS;
+int16_t g_qentry_wait = 0;
 #endif
 
 volatile spinlock_t g_iob_lock = SP_UNLOCKED;

diff --git a/mm/iob/iob_navail.c b/mm/iob/iob_navail.c
@@ -71,30 +71,3 @@ int iob_navail(bool throttled)
 
   return ret;
 }
-
-/****************************************************************************
- * Name: iob_qentry_navail
- *
- * Description:
- *   Return the number of available IOB chains.
- *
- ****************************************************************************/
-
-int iob_qentry_navail(void)
-{
-  int ret;
-
-#if CONFIG_IOB_NCHAINS > 0
-  /* Get the value of the IOB chain qentry counting semaphores */
-
-  ret = g_qentry_count;
-  if (ret < 0)
-    {
-      ret = 0;
-    }
-#else
-  ret = 0;
-#endif
-
-  return ret;
-}
diff --git a/mm/iob/iob_statistics.c b/mm/iob/iob_statistics.c
@@ -68,12 +68,8 @@ void iob_getstats(FAR struct iob_stats_s *stats)
     }
 
 #if CONFIG_IOB_THROTTLE > 0
-  stats->nthrottle = g_throttle_count;
+  stats->nthrottle = (g_iob_count - CONFIG_IOB_THROTTLE);
   if (stats->nthrottle < 0)
-    {
-      stats->nthrottle = -stats->nthrottle;
-    }
-  else
 #endif
     {
       stats->nthrottle = 0;