Merge 15756da9c3 into ddf350f1a3

fs/procfs/fs_procfsiobinfo.c

@@ -43,6 +43,7 @@
 #include <nuttx/mm/iob.h>
 #include <nuttx/fs/fs.h>
 #include <nuttx/fs/procfs.h>
+#include <nuttx/spinlock.h>
 
 #include "fs_heap.h"
 
@@ -111,6 +112,11 @@ const struct procfs_operations g_iobinfo_operations =
   iobinfo_stat    /* stat */
 };
 
+#ifdef CONFIG_IOB_OWNER_TRACKING
+extern volatile spinlock_t g_iob_lock;
+extern FAR struct iob_s *g_iob_committed;
+#endif
+
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
@@ -223,6 +229,44 @@ static ssize_t iobinfo_read(FAR struct file *filep, FAR char *buffer,
                              &offset);
   totalsize += copysize;
 
+#ifdef CONFIG_IOB_OWNER_TRACKING
+  buffer   += copysize;
+  buflen   -= copysize;
+
+  irqstate_t flags = spin_lock_irqsave(&g_iob_lock);
+
+  for (unsigned int i = 0; i < CONFIG_IOB_NBUFFERS; i++)
+    {
+      FAR struct iob_s *iob = iob_get_iob_by_index(i);
+
+      /* Print: PID (or -1 for ISR) and flags in hex */
+
+      if (iob->io_owner_flags != 0)
+        {
+          linesize = procfs_snprintf(iobfile->line, IOBINFO_LINELEN,
+                                    "%02d %p -> %p pid=%d flags=0x%02x\n", i,
+                                    iob, iob->io_flink,
+                                    (int)iob->io_owner_pid,
+                                    (unsigned)iob->io_owner_flags);
+        }
+      else
+        {
+          linesize = procfs_snprintf(iobfile->line, IOBINFO_LINELEN,
+                                    "%02d %p -> %p Free\n", i,
+                                    iob, iob->io_flink);
+        }
+
+      copysize = procfs_memcpy(iobfile->line, linesize,
+                               buffer, buflen, &offset);
+      totalsize += copysize;
+
+      buffer   += copysize;
+      buflen   -= copysize;
+    }
+
+  spin_unlock_irqrestore(&g_iob_lock, flags);
+#endif
+
   /* Update the file offset */
 
   filep->f_pos += totalsize;

include/nuttx/mm/iob.h

@@ -126,6 +126,12 @@ struct iob_s
 #endif
   unsigned int io_pktlen; /* Total length of the packet */
 
+ #ifdef CONFIG_IOB_OWNER_TRACKING
+  int16_t           io_owner_pid;
+  uint8_t           io_owner_cpu;   /* CPU index (SMP), 0 on UP */
+  uint8_t           io_owner_flags; /* bit0: ISR; bit1: committed; future use */
+#endif
+
 #ifdef CONFIG_IOB_ALLOC
   iob_free_cb_t io_free;  /* Custom free callback */
   FAR uint8_t  *io_data;
@@ -183,6 +189,10 @@ struct iob_stats_s
 
 void iob_initialize(void);
 
+#ifdef CONFIG_IOB_OWNER_TRACKING
+struct iob_s *iob_get_iob_by_index(unsigned int index);
+#endif
+
 /****************************************************************************
  * Name: iob_timedalloc
  *

mm/iob/Kconfig

@@ -122,5 +122,14 @@ config IOB_DEBUG
 		NOTE that this selection is not available if IOBs are being used
 		to syslog buffering logic (CONFIG_SYSLOG_BUFFER=y)!
 
+config IOB_OWNER_TRACKING
+	bool "Track task/ISR owner of each IOB (for debugging)"
+	default n
+	help
+		When enabled, each IOB carries small metadata indicating the
+		allocating context (task PID or ISR). This helps debug leaks,
+		starvation and ownership bugs without changing the IOB API.
+		Disabled by default to avoid extra RAM on small targets.
+
 endif # MM_IOB
 endmenu # Common I/O buffer support

mm/iob/iob_alloc.c

@@ -38,12 +38,34 @@
 #include <nuttx/nuttx.h>
 #include <nuttx/mm/iob.h>
 
+#ifdef CONFIG_IOB_OWNER_TRACKING
+#include <nuttx/irq.h>         /* up_interrupt_context() */
+#include <nuttx/sched.h>       /* getpid(), this_cpu() if available */
+#endif
+
 #include "iob.h"
 
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
+/* Small helper to stamp owner information */
+#ifdef CONFIG_IOB_OWNER_TRACKING
+static inline void iob_set_owner(FAR struct iob_s *iob, bool is_committed)
+{
+  bool isr = up_interrupt_context();
+  int  pid = isr ? -1 : getpid();           /* Kernel-space getpid is OK */
+ #ifdef CONFIG_SMP
+  iob->io_owner_cpu   = (uint8_t)this_cpu(); /* otherwise 0 on UP */
+ #else
+  iob->io_owner_cpu   = 0;
+ #endif
+  iob->io_owner_pid   = (int16_t)pid;
+  iob->io_owner_flags = 0x1 |
+  (isr ? 0x02 : 0x00) | (is_committed ? 0x04 : 0x00);
+}
+#endif
+
 static clock_t iob_allocwait_gettimeout(clock_t start, unsigned int timeout)
 {
   clock_t tick;
@@ -96,6 +118,9 @@ static FAR struct iob_s *iob_alloc_committed(void)
       iob->io_len    = 0;    /* Length of the data in the entry */
       iob->io_offset = 0;    /* Offset to the beginning of data */
       iob->io_pktlen = 0;    /* Total length of the packet */
+#ifdef CONFIG_IOB_OWNER_TRACKING
+      iob_set_owner(iob, true);
+#endif
     }
 
   spin_unlock_irqrestore(&g_iob_lock, flags);
@@ -135,6 +160,11 @@ static FAR struct iob_s *iob_tryalloc_internal(bool throttled)
           iob->io_len    = 0;    /* Length of the data in the entry */
           iob->io_offset = 0;    /* Offset to the beginning of data */
           iob->io_pktlen = 0;    /* Total length of the packet */
+
+#ifdef CONFIG_IOB_OWNER_TRACKING
+          iob_set_owner(iob, false);
+#endif
+
           return iob;
         }
     }

mm/iob/iob_free.c

@@ -133,6 +133,13 @@ FAR struct iob_s *iob_free(FAR struct iob_s *iob)
           kmm_free(iob);
         }
 
+#ifdef CONFIG_IOB_OWNER_TRACKING
+      /* Clear owner metadata when returning to free list */
+
+      iob->io_owner_pid   = 0;
+      iob->io_owner_cpu   = 0;
+      iob->io_owner_flags = 0;
+#endif
       return next;
     }
 #endif
@@ -196,6 +203,14 @@ FAR struct iob_s *iob_free(FAR struct iob_s *iob)
     }
 #endif
 
+#ifdef CONFIG_IOB_OWNER_TRACKING
+  /* Clear owner metadata when returning to free list */
+
+  iob->io_owner_pid   = 0;
+  iob->io_owner_cpu   = 0;
+  iob->io_owner_flags = 0;
+#endif
+
   /* And return the I/O buffer after the one that was freed */
 
   return next;

mm/iob/iob_free.c.orig

@@ -0,0 +1,222 @@
+/****************************************************************************
+ * mm/iob/iob_free.c
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdbool.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+#ifdef CONFIG_IOB_ALLOC
+#  include <nuttx/kmalloc.h>
+#endif
+#include <nuttx/nuttx.h>
+#include <nuttx/mm/iob.h>
+
+#include "iob.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#ifdef CONFIG_IOB_NOTIFIER
+#  if !defined(CONFIG_IOB_NOTIFIER_DIV) || CONFIG_IOB_NOTIFIER_DIV < 2
+#    define IOB_DIVIDER 1
+#  elif CONFIG_IOB_NOTIFIER_DIV < 4
+#    define IOB_DIVIDER 2
+#  elif CONFIG_IOB_NOTIFIER_DIV < 8
+#    define IOB_DIVIDER 4
+#  elif CONFIG_IOB_NOTIFIER_DIV < 16
+#    define IOB_DIVIDER 8
+#  elif CONFIG_IOB_NOTIFIER_DIV < 32
+#    define IOB_DIVIDER 16
+#  elif CONFIG_IOB_NOTIFIER_DIV < 64
+#    define IOB_DIVIDER 32
+#  else
+#    define IOB_DIVIDER 64
+#  endif
+#endif
+
+#define IOB_MASK      (IOB_DIVIDER - 1)
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: iob_free
+ *
+ * Description:
+ *   Free the I/O buffer at the head of a buffer chain returning it to the
+ *   free list.  The link to  the next I/O buffer in the chain is return.
+ *
+ ****************************************************************************/
+
+FAR struct iob_s *iob_free(FAR struct iob_s *iob)
+{
+  FAR struct iob_s *next = iob->io_flink;
+  irqstate_t flags;
+#ifdef CONFIG_IOB_NOTIFIER
+  int16_t navail;
+#endif
+
+  iobinfo("iob=%p io_pktlen=%u io_len=%u next=%p\n",
+          iob, iob->io_pktlen, iob->io_len, next);
+
+  /* Copy the data that only exists in the head of a I/O buffer chain into
+   * the next entry.
+   */
+
+  if (next != NULL)
+    {
+      /* Copy and decrement the total packet length, being careful to
+       * do nothing too crazy.
+       */
+
+      if (iob->io_pktlen > iob->io_len)
+        {
+          /* Adjust packet length and move it to the next entry */
+
+          next->io_pktlen = iob->io_pktlen - iob->io_len;
+          DEBUGASSERT(next->io_pktlen >= next->io_len);
+        }
+      else
+        {
+          /* This can only happen if the free entry isn't first entry in the
+           * chain...
+           */
+
+          next->io_pktlen = 0;
+        }
+
+      iobinfo("next=%p io_pktlen=%u io_len=%u\n",
+              next, next->io_pktlen, next->io_len);
+    }
+
+#ifdef CONFIG_IOB_ALLOC
+  if (iob->io_free != NULL)
+    {
+<<<<<<< HEAD
+      FAR uint8_t *io_data = (FAR uint8_t *)ALIGN_UP((uintptr_t)(iob + 1),
+                                                     IOB_ALIGNMENT);
+      if (iob->io_data == io_data)
+        {
+          iob->io_free(iob);
+        }
+      else
+        {
+          iob->io_free(iob->io_data);
+          kmm_free(iob);
+        }
+
+=======
+      iob->io_free(iob->io_data);
+#ifdef CONFIG_IOB_OWNER_TRACKING
+      /* Clear owner metadata when returning to free list */
+
+      iob->io_owner_pid   = 0;
+      iob->io_owner_cpu   = 0;
+      iob->io_owner_flags = 0;
+#endif
+      kmm_free(iob);
+>>>>>>> b07e949139 (net/iob: Add CONFIG_IOB_OWNER_TRACKING to record IOB owner for debugging)
+      return next;
+    }
+#endif
+
+  /* Free the I/O buffer by adding it to the head of the free or the
+   * committed list. 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.
+   */
+
+  flags = spin_lock_irqsave(&g_iob_lock);
+
+  /* Which list?  If there is a task waiting for an IOB, then put
+   * the IOB on either the free list or on the committed list where
+   * it is reserved for that allocation (and not available to
+   * iob_tryalloc()). This is true for both throttled and non-throttled
+   * cases.
+   */
+
+  if (g_iob_count < 0)
+    {
+      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
+  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(&g_throttle_sem);
+    }
+#endif
+  else
+    {
+      g_iob_count++;
+      iob->io_flink   = g_iob_freelist;
+      g_iob_freelist  = iob;
+      spin_unlock_irqrestore(&g_iob_lock, flags);
+    }
+
+  DEBUGASSERT(g_iob_count <= CONFIG_IOB_NBUFFERS);
+
+#ifdef CONFIG_IOB_NOTIFIER
+  /* Check if the IOB was claimed by a thread that is blocked waiting
+   * for an IOB.
+   */
+
+  navail = iob_navail(false);
+  if (navail > 0 && (navail & IOB_MASK) == 0)
+    {
+      /* Signal any threads that have requested a signal notification
+       * when an IOB becomes available.
+       */
+
+      iob_notifier_signal();
+    }
+#endif
+
+#ifdef CONFIG_IOB_OWNER_TRACKING
+  /* Clear owner metadata when returning to free list */
+
+  iob->io_owner_pid   = 0;
+  iob->io_owner_cpu   = 0;
+  iob->io_owner_flags = 0;
+#endif
+
+  /* And return the I/O buffer after the one that was freed */
+
+  return next;
+}

mm/iob/iob_initialize.c

@@ -175,3 +175,20 @@ void iob_initialize(void)
     }
 #endif
 }
+
+#ifdef CONFIG_IOB_OWNER_TRACKING
+struct iob_s *iob_get_iob_by_index(unsigned int index)
+{
+  uintptr_t buf;
+
+  if (index > CONFIG_IOB_NBUFFERS)
+    {
+      return 0;
+    }
+
+  buf = ALIGN_UP((uintptr_t)g_iob_buffer + offsetof(struct iob_s, io_data),
+                CONFIG_IOB_ALIGNMENT) - offsetof(struct iob_s, io_data);
+
+  return (FAR struct iob_s *)(buf + index * IOB_ALIGN_SIZE);
+}
+#endif