git: e75b324c93a1 - main - kern_descrip.c: Clarify allocation and freeing of fd map in fdgrowtable()

The branch main has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=e75b324c93a14ab68d79d9247943ae10da184657

commit e75b324c93a14ab68d79d9247943ae10da184657
Author:     Kristofer Peterson <kris@tranception.com>
AuthorDate: 2026-02-18 13:56:53 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2026-04-16 06:05:22 +0000

    kern_descrip.c: Clarify allocation and freeing of fd map in fdgrowtable()
    
    When expanding a file table, the condition for allocating a new map
    is NDSLOTS(nnfiles) > NDSLOTS(onfiles) whereas for freeing the old map
    is NDSLOTS(onfiles) > NDSLOTS(NDFILE).
    
    If a previously expanded file table were to be expanded slightly again
    such that the map did not need to be increased, then fdgrowtable could
    still free the current map.
    
    This does not happen currently as nnfiles is rounded up to a multiple of
    NDENTRIES at the beginning of fdgrowtable() so that every enlargement
    after the first enlargement will always require a larger map.
    
    Though the logic is currently correct, it is unclear and should the
    earlier rounding up of nnfiles be relaxed or remove, the logic would
    be incorrect. This patch therefore adds comments and invariants checking
    the size of the table and map, and updates the map free condition so
    that it is absolutely clear that the old map will only be deallocated
    if a new map has been allocated.
    
    Signed-off-by: Kristofer Peterson <kris@tranception.com>
    Reviewed by: kib, kevans
    Pull Request: https://github.com/freebsd/freebsd-src/pull/2029
---
 sys/kern/kern_descrip.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 2fa0621bdfca..69985c39c3c0 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -2006,6 +2006,21 @@ fdgrowtable(struct filedesc *fdp, int nfd)
 	NDSLOTTYPE *nmap, *omap;
 
 	KASSERT(fdp->fd_nfiles > 0, ("zero-length file table"));
+	KASSERT(fdp->fd_nfiles >= NDFILE, ("file table of length %d shorter "
+	    "than NDFILE (%d)", fdp->fd_nfiles, NDFILE));
+	KASSERT(fdp->fd_nfiles == NDFILE || fdp->fd_nfiles % NDENTRIES == 0,
+	    ("file table of length %d should be multiple of NDENTRIES (%lu)",
+	    fdp->fd_nfiles, NDENTRIES));
+	KASSERT((fdp->fd_nfiles == NDFILE) == ((intptr_t)fdp->fd_files -
+	    offsetof(struct filedesc0, fd_dfiles) == (intptr_t)fdp -
+	    offsetof(struct filedesc0, fd_fd)), ("file table of length %d "
+	    "should have %s table", fdp->fd_nfiles, fdp->fd_nfiles == NDFILE ?
+	    "initial" : "dynamic"));
+	KASSERT((NDSLOTS(fdp->fd_nfiles) <= NDSLOTS(NDFILE)) == ((intptr_t)
+	    fdp->fd_map - offsetof(struct filedesc0, fd_dmap) == (intptr_t)fdp -
+	    offsetof(struct filedesc0, fd_fd)), ("file table of length %d "
+	    "should have %s map", fdp->fd_nfiles, NDSLOTS(fdp->fd_nfiles) <=
+	    NDSLOTS(NDFILE) ? "initial" : "dynamic"));
 
 	/* save old values */
 	onfiles = fdp->fd_nfiles;
@@ -2035,9 +2050,19 @@ fdgrowtable(struct filedesc *fdp, int nfd)
 	    onfiles * sizeof(ntable->fdt_ofiles[0]));
 
 	/*
-	 * Allocate a new map only if the old is not large enough.  It will
-	 * grow at a slower rate than the table as it can map more
-	 * entries than the table can hold.
+	 * Allocate a new map only if the old one is not large enough.
+	 *
+	 * The initial struct filedesc0 object contains a table and map sized
+	 * for NDFILE (20) entries which means the initial map can accomodate
+	 * up to NDENTRIES (32 or 64) before requiring reallocation.
+	 *
+	 * As the new table size (nnfiles) is always rounded up to a multiple
+	 * of NDENTRIES, the map will be fully utilised following the first
+	 * enlargement, whether it is still the initial map (which will be the
+	 * case if nnfiles == NDENTRIES) or if a new one that has has been
+	 * allocated (which will be the case if nnfiles == X*NDENTRIES for some
+	 * X > 1). In either case, subsequent enlargements will always allocate
+	 * a new map to go along with the new table.
 	 */
 	if (NDSLOTS(nnfiles) > NDSLOTS(onfiles)) {
 		nmap = malloc(NDSLOTS(nnfiles) * NDSLOTSIZE, M_FILEDESC,
@@ -2045,6 +2070,8 @@ fdgrowtable(struct filedesc *fdp, int nfd)
 		/* copy over the old data and update the pointer */
 		memcpy(nmap, omap, NDSLOTS(onfiles) * sizeof(*omap));
 		fdp->fd_map = nmap;
+	} else {
+		nmap = NULL;
 	}
 
 	/*
@@ -2085,9 +2112,10 @@ fdgrowtable(struct filedesc *fdp, int nfd)
 	/*
 	 * The map does not have the same possibility of threads still
 	 * holding references to it.  So always free it as long as it
-	 * does not reference the original static allocation.
+	 * does not reference the original static allocation and a new
+	 * map was allocated.
 	 */
-	if (NDSLOTS(onfiles) > NDSLOTS(NDFILE))
+	if (nmap != NULL && NDSLOTS(onfiles) > NDSLOTS(NDFILE))
 		free(omap, M_FILEDESC);
 }