diff --git a/fs/procfs/fs_procfsiobinfo.c b/fs/procfs/fs_procfsiobinfo.c index 6623806751580..b9bd6636343ae 100644 --- a/fs/procfs/fs_procfsiobinfo.c +++ b/fs/procfs/fs_procfsiobinfo.c @@ -43,6 +43,7 @@ #include #include #include +#include #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; diff --git a/include/nuttx/mm/iob.h b/include/nuttx/mm/iob.h index 4c3c2710264ae..b5a5e81fd93e4 100644 --- a/include/nuttx/mm/iob.h +++ b/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 * diff --git a/mm/iob/Kconfig b/mm/iob/Kconfig index 6dc8ea4ea3ea7..d1a0c92e8e800 100644 --- a/mm/iob/Kconfig +++ b/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 diff --git a/mm/iob/iob_alloc.c b/mm/iob/iob_alloc.c index 130526bc49fcc..25cfdc3a0f9d5 100644 --- a/mm/iob/iob_alloc.c +++ b/mm/iob/iob_alloc.c @@ -38,12 +38,34 @@ #include #include +#ifdef CONFIG_IOB_OWNER_TRACKING +#include /* up_interrupt_context() */ +#include /* 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) { sclock_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; } } diff --git a/mm/iob/iob_free.c b/mm/iob/iob_free.c index 4c5d8e6e0a850..58def88e6dbb5 100644 --- a/mm/iob/iob_free.c +++ b/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; diff --git a/mm/iob/iob_free.c.orig b/mm/iob/iob_free.c.orig new file mode 100644 index 0000000000000..122997ced9af3 --- /dev/null +++ b/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 + +#include +#include +#include + +#include +#include +#ifdef CONFIG_IOB_ALLOC +# include +#endif +#include +#include + +#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; +} diff --git a/mm/iob/iob_initialize.c b/mm/iob/iob_initialize.c index 02aad9bbfe0bf..a04721290a8dc 100644 --- a/mm/iob/iob_initialize.c +++ b/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