Date: Tue, 18 Sep 2012 17:46:23 +0200 From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@des.no> To: hackers@freebsd.org Subject: fdgrowtable() cleanup Message-ID: <86wqzr8hbk.fsf@ds4.des.no>
next in thread | raw e-mail | index | archive | help
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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86wqzr8hbk.fsf>