From owner-freebsd-hackers@FreeBSD.ORG Tue Sep 18 15:46:26 2012 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id A50101065673 for ; Tue, 18 Sep 2012 15:46:26 +0000 (UTC) (envelope-from des@des.no) Received: from smtp.des.no (smtp.des.no [194.63.250.102]) by mx1.freebsd.org (Postfix) with ESMTP id 69FA98FC16 for ; Tue, 18 Sep 2012 15:46:26 +0000 (UTC) Received: from ds4.des.no (smtp.des.no [194.63.250.102]) by smtp.des.no (Postfix) with ESMTP id 0089B6BFB for ; Tue, 18 Sep 2012 17:46:25 +0200 (CEST) Received: by ds4.des.no (Postfix, from userid 1001) id 8F58E837C; Tue, 18 Sep 2012 17:46:24 +0200 (CEST) From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= To: hackers@freebsd.org Date: Tue, 18 Sep 2012 17:46:23 +0200 Message-ID: <86wqzr8hbk.fsf@ds4.des.no> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.4 (berkeley-unix) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: Subject: fdgrowtable() cleanup X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Sep 2012 15:46:26 -0000 The patch below my signature improves the legibility of fdgrowtable(), and adds comments explaining the hairier bits. The only functional change is that the code no longer overwrites the old fileflags array when the old table is placed on the freelist; instead, it uses the space actually set aside for that purpose. (I assume that the old behavior was harmless, since it has persisted for decades, but it was certainly confusing.) The slightly repetitive nature of the new code is intentional. DES --=20 Dag-Erling Sm=C3=B8rgrav - des@des.no Index: sys/kern/kern_descrip.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/kern/kern_descrip.c (revision 240654) +++ sys/kern/kern_descrip.c (working copy) @@ -148,11 +148,6 @@ #define NDSLOTS(x) (((x) + NDENTRIES - 1) / NDENTRIES) =20 /* - * Storage required per open file descriptor. - */ -#define OFILESIZE (sizeof(struct file *) + sizeof(char)) - -/* * Storage to hold unused ofiles that need to be reclaimed. */ struct freetable { @@ -1436,7 +1431,7 @@ struct freetable *fo; struct file **ntable; struct file **otable; - char *nfileflags; + char *nfileflags, *ofileflags; int nnfiles, onfiles; NDSLOTTYPE *nmap; =20 @@ -1447,33 +1442,46 @@ =20 /* compute the size of the new table */ onfiles =3D fdp->fd_nfiles; + otable =3D fdp->fd_ofiles; + ofileflags =3D fdp->fd_ofileflags; nnfiles =3D NDSLOTS(nfd) * NDENTRIES; /* round up */ if (nnfiles <=3D onfiles) /* the table is already large enough */ return; =20 - /* allocate a new table and (if required) new bitmaps */ - ntable =3D malloc((nnfiles * OFILESIZE) + sizeof(struct freetable), + /* + * Allocate a new table. We need enough space for a) the file + * entries themselves, b) the file flags, and c) the struct + * freetable we will use when we decommission the table and place + * it on the freelist. + */ + ntable =3D malloc(nnfiles * sizeof(*ntable) + + nnfiles * sizeof(*nfileflags) + + sizeof(struct freetable), M_FILEDESC, M_ZERO | M_WAITOK); nfileflags =3D (char *)&ntable[nnfiles]; + + /* allocate new bitmaps if necessary */ if (NDSLOTS(nnfiles) > NDSLOTS(onfiles)) nmap =3D malloc(NDSLOTS(nnfiles) * NDSLOTSIZE, M_FILEDESC, M_ZERO | M_WAITOK); else nmap =3D NULL; =20 - bcopy(fdp->fd_ofiles, ntable, onfiles * sizeof(*ntable)); - bcopy(fdp->fd_ofileflags, nfileflags, onfiles); - otable =3D fdp->fd_ofiles; + /* copy the old data over and point at the new tables */ + bcopy(otable, ntable, onfiles * sizeof(*otable)); + bcopy(ofileflags, nfileflags, onfiles * sizeof(*ofileflags)); fdp->fd_ofileflags =3D nfileflags; fdp->fd_ofiles =3D ntable; + /* * We must preserve ofiles until the process exits because we can't * be certain that no threads have references to the old table via * _fget(). */ if (onfiles > NDFILE) { - fo =3D (struct freetable *)&otable[onfiles]; + fo =3D (struct freetable *)(char *)otable + + onfiles * sizeof(*otable) + onfiles * sizeof(*ofileflags); fdp0 =3D (struct filedesc0 *)fdp; fo->ft_table =3D otable; SLIST_INSERT_HEAD(&fdp0->fd_free, fo, ft_next);