Date: Wed, 9 May 2001 13:33:54 -0700 (PDT) From: Matt Dillon <dillon@earth.backplane.com> To: Seigo Tanimura <tanimura@r.dl.itc.u-tokyo.ac.jp> Cc: jhb@FreeBSD.ORG, current@FreeBSD.ORG Subject: Re: RE: select(2) converted to use a condition variable, and optimis Message-ID: <200105092033.f49KXsB00219@earth.backplane.com> References: <200105081026.f48AQgP75260@rina.r.dl.itc.u-tokyo.ac.jp> <XFMail.010508082155.jhb@FreeBSD.org> <200105091020.f49AK7P05497@rina.r.dl.itc.u-tokyo.ac.jp>
next in thread | previous in thread | raw e-mail | index | archive | help
There are several issues here:
* The process's descriptor table
* The struct file's referenced by that descriptor table
* The object underlying a struct file.
A process's descriptor table is not protected by the proc lock, because
the descriptor table can be shared across processes that rfork().
The struct file underlying a descriptor has a reference count. The
descriptor table reference to the underlying struct file is part of
this ref count. Any process that fhold()'s a struct file pointer also
bumps this ref count. However, holding a ref on the struct file does
not prevent another process from revoking the descriptor, so you aren't
as safe as you might think. You are safe from it being closed out
from under you, but not safe otherwise.
Additionally, the act of *getting* a new ref count on a struct file is
not currently protected by anything but Giant. For example, look
at kern/kern_descrip.c line 226 in -current:
if ((unsigned)uap->fd >= fdp->fd_nfiles ||
(fp = fdp->fd_ofiles[uap->fd]) == NULL)
return (EBADF);
pop = &fdp->fd_ofileflags[uap->fd];
...
fhold(fp);
A race can develope between the point where 'fp' is loaded, and the
point where it is held by fhold(). Linux solves this problem by making
their fhold() equivalent take a descriptor number rather then a struct
file and doing all necessary locking internally.
I did a bunch of work on the descriptor code to handle -stable SMP
races that were occuring due to a lack of fhold()s around blocking
conditions, but I never took the final step of encapsulating the
descriptor number lookup in the fhold() (I ran out of time).
For -current, you MUST do this if you want to move any
descriptor-referencing code out from under Giant. The process lock
is not sufficient... the descriptor table needs its own mutex. It
is possible to optimize this out by checking to see if the descriptor
table is shared, but in general you need it.
-Matt
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200105092033.f49KXsB00219>