Date: Sun, 24 Sep 2000 12:53:12 -0700 From: Alfred Perlstein <bright@wintelcom.net> To: Bosko Milekic <bmilekic@technokratis.com> Cc: arch@FreeBSD.ORG, cp@FreeBSD.ORG Subject: Re: need advice, fsetown annoyances and mpsafeness. Message-ID: <20000924125311.Q9141@fw.wintelcom.net> In-Reply-To: <Pine.BSF.4.21.0009241420280.14398-100000@jehovah.technokratis.com>; from bmilekic@technokratis.com on Sun, Sep 24, 2000 at 02:37:52PM -0400 References: <20000924103303.M9141@fw.wintelcom.net> <Pine.BSF.4.21.0009241420280.14398-100000@jehovah.technokratis.com>
next in thread | previous in thread | raw e-mail | index | archive | help
* Bosko Milekic <bmilekic@technokratis.com> [000924 11:34] wrote:
>
>
> On Sun, 24 Sep 2000, Alfred Perlstein wrote:
>
> > What it then does is walk through the sigio structs hung from itself
> > and using a back-pointer that points to the pointer within the
> > object (socket/tty) it raises splhigh and NULLs it out, lowers spl,
> > then frees the sigio.
> >
> > s = splhigh();
> > *(sigio->sio_myref) = NULL;
> > splx(s);
>
> Why can't this be done with an atomic operation? If you're holding
> the sigio struct, then are you not also ensuring that sigio->sio_myref
> won't change. Setting the pointer within the object to NULL should be
> atomic in itself, AFAIK. I'm wondering what would happen if the object is
> destroyed just before you splhigh() up there (in other words, did you
> leave something out of the example you posted above?)
> Assuming something was left out, then I'm wondering if it would be
> profitable in this case to distinguish between the nature of the object
> and optionally provide a pointer to a mutex in the sigio struct which
> should be aquired in order to do this manipulation.
It's really a lot more evil than you think.
The race is in the object (socket/tty) checking the pointer and
then dereferencing it.
A broken solution is to lock the sigio struct or provide a backreference
to the socket/tty lock, after banging my head against my desk for some time I came across this solution:
(assuming pfind/pgfind return the proc/pgrp locked)
/*
* called by the owner of a sigio struct such as a tty/socket to remove
* a struct sigio from itself, called at object destruction or at the
* the time that sigio/sigurg is no longer wanted/needed
* it will lock and unlock the proc/pgrp target of the sigio
*/
void
funsetown_obj(sigio)
struct sigio *sigio;
{
pid_t pid;
if (sigio == NULL)
return;
/*
* ok this is somewhat tricky, we examine what the sigio is attached
* to, whatever it is proc/pgrp we need to use the search functions
* to ensure atomicity. If we get back ESRCH that's ok, that means
* we lost the race, just free it.
* if we get back a pointer we then need to make sure that the pgid
* hasn't been NULLed out because we lost the race between looking
* at the sigio and locking the proc/pgrp
* (most likely pid/pgid wraparound)
*/
pid = sigio->sio_pgid;
if (pid < 0) {
struct pgrp *p;
if ((pgrp = pgfind(pid)) != NULL) {
/* funsetown_proc would have set this to zero */
if (sigio->sio_pgid != 0)
SLIST_REMOVE(&sigio->sio_pgrp->pg_sigiolst, sigio,
sigio, sio_pgsigio);
PGRP_UNLOCK(&sigio->sio_pgrp);
}
} else if (pid > 0) {
struct proc *p;
if ((p = pfind(pid)) != NULL) {
/* funsetown_proc would have set this to zero */
if (sigio->sio_pgid != 0)
SLIST_REMOVE(&sigio->sio_proc->p_sigiolst, sigio,
sigio, sio_pgsigio);
PROC_UNLOCK(&sigio->sio_proc);
}
}
out:
crfree(sigio->sio_ucred);
FREE(sigio, M_SIGIO);
}
/*
* NULL out a sigio struct attached to a process/pgrp
* must be called with the object (struct proc/pgrp) locked
* this is to be called from the perspective of the process/pgrp
*
* called from the proc/pgid at teardown
* proc/pgid must be locked
*/
void
funsetown_proc(sigio)
struct sigio *sigio;
{
int s;
if (sigio == NULL)
return;
if (sigio->sio_pgid < 0) {
SLIST_REMOVE(&sigio->sio_pgrp->pg_sigiolst, sigio,
sigio, sio_pgsigio);
} else /* if ((*sigiop)->sio_pgid > 0) */ {
SLIST_REMOVE(&sigio->sio_proc->p_sigiolst, sigio,
sigio, sio_pgsigio);
}
sigio->sio_pgid = 0;
}
/*
* Free a list of sigio structures.
*
* called from the proc/pgid at teardown
* proc/pgid must be locked
*/
void
funsetownlst(sigiolst)
struct sigiolst *sigiolst;
{
struct sigio *sigio;
while ((sigio = SLIST_FIRST(sigiolst)) != NULL)
funsetown(sigio);
}
Questions? Comments?
--
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
"I have the heart of a child; I keep it in a jar on my desk."
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20000924125311.Q9141>