--ELM716650414-2352-0_
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=US-ASCII

Chuck Silvers wrote:
> in the part of my previous mail after the part you quoted below,
> I explained how you can avoid using kmem_map space even for allocations
> smaller than a page.  if you use that mechanism for allocating
> file descriptor structures, then there would not be any way to
> have problems with malloc() failing in this path.  we should use
> that same mechanism for all memory allocations where we currently
> use malloc() to handle a variably-sized allocation but we don't need
> the interrupt-safe property.

Yes, it could be done using the pools, but it feels a bit icky to
me. It doesn't seem to be possible to stick this into current
malloc(9) and I don't think it's very reasonable to add completely
new nointr subpage allocator.  Given these practical considerations,
it seems best to just use malloc() (and thus kmem_map) for <PAGE_SIZE
size, and uvm_km_alloc() for >= PAGE_SIZE.

I'm appending patch which does just this. It also adds change to kill
the program if memory allocation for file descriptor array fails.

Opinions?

Jaromir
-- 
Jaromir Dolecek <jdolecek@NetBSD.org> http://www.NetBSD.org/Ports/i386/ps2.html
-=  Those who would give up liberty for a little temporary safety deserve  =-
-=  neither liberty nor safety, and will lose both.  -- Benjamin Franklin  =-

--ELM716650414-2352-0_
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=ISO-8859-2
Content-Disposition: attachment; filename=fdexp.diff

Index: kern_descrip.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/kern_descrip.c,v
retrieving revision 1.83
diff -u -p -r1.83 kern_descrip.c
--- kern_descrip.c	2001/12/07 07:09:29	1.83
+++ kern_descrip.c	2002/01/08 21:01:45
@@ -61,6 +61,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_descrip
 #include <sys/unistd.h>
 #include <sys/resourcevar.h>
 #include <sys/conf.h>
+#include <sys/wait.h>
 
 #include <sys/mount.h>
 #include <sys/syscallargs.h>
@@ -642,6 +643,7 @@ fdexpand(struct proc *p)
 	int		i, nfiles;
 	struct file	**newofile;
 	char		*newofileflags;
+	vsize_t		len;
 
 	fdp = p->p_fd;
 
@@ -649,21 +651,39 @@ fdexpand(struct proc *p)
 		nfiles = NDEXTENT;
 	else
 		nfiles = 2 * fdp->fd_nfiles;
-	newofile = malloc(nfiles * OFILESIZE, M_FILEDESC, M_WAITOK);
+
+	if (__predict_true((len = nfiles * OFILESIZE) < PAGE_SIZE))
+		newofile = malloc(len, M_FILEDESC, M_WAITOK|M_CANFAIL|M_ZERO);
+	else {
+		newofile = (struct file **) uvm_km_alloc(kernel_map,
+		    round_page(len));
+	}
+
+	if (__predict_false(newofile == NULL)) {
+		int uid = p->p_cred && p->p_ucred ? p->p_ucred->cr_uid : -1;
+		log(LOG_INFO, "pid %d (%s), uid %d: fdexpand() to %d files failed, process killed",
+			p->p_pid, p->p_comm, uid, nfiles);
+		exit1(p, W_EXITCODE(0, SIGKILL));
+		/* NOTREACHED */
+	}
+
 	newofileflags = (char *) &newofile[nfiles];
 	/*
-	 * Copy the existing ofile and ofileflags arrays
-	 * and zero the new portion of each array.
+	 * Copy the existing ofile and ofileflags arrays.
+	 * The new portion of each array were zeroed above.
 	 */
 	memcpy(newofile, fdp->fd_ofiles,
-		(i = sizeof(struct file *) * fdp->fd_nfiles));
-	memset((char *)newofile + i, 0,
-	    nfiles * sizeof(struct file *) - i);
+	    (i = sizeof(struct file *) * fdp->fd_nfiles));
 	memcpy(newofileflags, fdp->fd_ofileflags,
 	    (i = sizeof(char) * fdp->fd_nfiles));
-	memset(newofileflags + i, 0, nfiles * sizeof(char) - i);
-	if (fdp->fd_nfiles > NDFILE)
-		free(fdp->fd_ofiles, M_FILEDESC);
+	if (fdp->fd_nfiles > NDFILE) {
+		if (__predict_true((fdp->fd_nfiles * OFILESIZE) < PAGE_SIZE))
+			free(fdp->fd_ofiles, M_FILEDESC);
+		else {
+			uvm_km_free(kernel_map, (vaddr_t) fdp->fd_ofiles,
+			    round_page(fdp->fd_nfiles * OFILESIZE));
+		}
+	}
 	fdp->fd_ofiles = newofile;
 	fdp->fd_ofileflags = newofileflags;
 	fdp->fd_nfiles = nfiles;

--ELM716650414-2352-0_--