Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 2 Mar 2014 16:25:56 +0000 (UTC)
From:      Bryan Drewery <bdrewery@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r262693 - stable/10/sys/kern
Message-ID:  <201403021625.s22GPuUI055001@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: bdrewery
Date: Sun Mar  2 16:25:56 2014
New Revision: 262693
URL: http://svnweb.freebsd.org/changeset/base/262693

Log:
  MFC r262006,r262328:
  
   r262006:
    Fix M_FILEDESC leak in fdgrowtable() introduced in r244510.
    fdgrowtable() now only reallocates fd_map when necessary.
   r262328:
    Style.
  
  Approved by:	bapt (mentor, implicit)

Modified:
  stable/10/sys/kern/kern_descrip.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/kern/kern_descrip.c
==============================================================================
--- stable/10/sys/kern/kern_descrip.c	Sun Mar  2 16:04:27 2014	(r262692)
+++ stable/10/sys/kern/kern_descrip.c	Sun Mar  2 16:25:56 2014	(r262693)
@@ -1522,7 +1522,7 @@ fdgrowtable(struct filedesc *fdp, int nf
 		return;
 
 	/*
-	 * Allocate a new table and map.  We need enough space for the
+	 * Allocate a new table.  We need enough space for the
 	 * file entries themselves and the struct freetable we will use
 	 * when we decommission the table and place it on the freelist.
 	 * We place the struct freetable in the middle so we don't have
@@ -1530,16 +1530,22 @@ fdgrowtable(struct filedesc *fdp, int nf
 	 */
 	ntable = malloc(nnfiles * sizeof(ntable[0]) + sizeof(struct freetable),
 	    M_FILEDESC, M_ZERO | M_WAITOK);
-	nmap = malloc(NDSLOTS(nnfiles) * NDSLOTSIZE, M_FILEDESC,
-	    M_ZERO | M_WAITOK);
-
 	/* copy the old data over and point at the new tables */
 	memcpy(ntable, otable, onfiles * sizeof(*otable));
-	memcpy(nmap, omap, NDSLOTS(onfiles) * sizeof(*omap));
-
-	/* update the pointers and counters */
 	fdp->fd_ofiles = ntable;
-	fdp->fd_map = nmap;
+
+	/*
+	 * 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.
+	 */
+	if (NDSLOTS(nnfiles) > NDSLOTS(onfiles)) {
+		nmap = malloc(NDSLOTS(nnfiles) * NDSLOTSIZE, M_FILEDESC,
+		    M_ZERO | M_WAITOK);
+		/* copy over the old data and update the pointer */
+		memcpy(nmap, omap, NDSLOTS(onfiles) * sizeof(*omap));
+		fdp->fd_map = nmap;
+	}
 
 	/*
 	 * In order to have a valid pattern for fget_unlocked()
@@ -1555,8 +1561,6 @@ fdgrowtable(struct filedesc *fdp, int nf
 	 * reference entries within it.  Instead, place it on a freelist
 	 * which will be processed when the struct filedesc is released.
 	 *
-	 * Do, however, free the old map.
-	 *
 	 * Note that if onfiles == NDFILE, we're dealing with the original
 	 * static allocation contained within (struct filedesc0 *)fdp,
 	 * which must not be freed.
@@ -1566,8 +1570,14 @@ fdgrowtable(struct filedesc *fdp, int nf
 		fdp0 = (struct filedesc0 *)fdp;
 		ft->ft_table = otable;
 		SLIST_INSERT_HEAD(&fdp0->fd_free, ft, ft_next);
-		free(omap, M_FILEDESC);
 	}
+	/*
+	 * 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.
+	 */
+	if (NDSLOTS(onfiles) > NDSLOTS(NDFILE))
+		free(omap, M_FILEDESC);
 }
 
 /*



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201403021625.s22GPuUI055001>