From owner-freebsd-arch Sun Sep 24 12:53:25 2000 Delivered-To: freebsd-arch@freebsd.org Received: from fw.wintelcom.net (ns1.wintelcom.net [209.1.153.20]) by hub.freebsd.org (Postfix) with ESMTP id DB4A637B422; Sun, 24 Sep 2000 12:53:13 -0700 (PDT) Received: (from bright@localhost) by fw.wintelcom.net (8.10.0/8.10.0) id e8OJrCI01203; Sun, 24 Sep 2000 12:53:12 -0700 (PDT) Date: Sun, 24 Sep 2000 12:53:12 -0700 From: Alfred Perlstein To: Bosko Milekic Cc: arch@FreeBSD.ORG, cp@FreeBSD.ORG Subject: Re: need advice, fsetown annoyances and mpsafeness. Message-ID: <20000924125311.Q9141@fw.wintelcom.net> References: <20000924103303.M9141@fw.wintelcom.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.4i In-Reply-To: ; from bmilekic@technokratis.com on Sun, Sep 24, 2000 at 02:37:52PM -0400 Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG * Bosko Milekic [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