RE: selwakeup()

2001-04-05 Thread John Baldwin


On 05-Apr-01 Garrett Wollman wrote:
 On Thu, 05 Apr 2001 01:39:35 -0500, Richard Todd
 [EMAIL PROTECTED] said:
 
 If I'm reading this backtrace right, the thread handling the sound
 hardware called selwakeup() (frame #19).  This called pfind() (frame
 #18), which tries to lock allproc.
 
 selwakeup() shouldn't need to call pfind().  Because the process table
 is in type-stable memory, it should be sufficient to keep a reference
 to the caller's proc structure and check to see whether its pid is the
 same one as in the selinfo.  The locking that selwakeup() already
 needs to do should be sufficient to avoid a race.
 
 (In 4.4BSD, process structures were not type-stable so this technique
 could not have been used.)

There are probably several other places that pfind is called that this check
should also be adequate for as well.  The ones in syscons for example.

 -GAWollman

-- 

John Baldwin [EMAIL PROTECTED] -- http://www.FreeBSD.org/~jhb/
PGP Key: http://www.baldwin.cx/~john/pgpkey.asc
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: selwakeup()

2001-04-05 Thread John Baldwin


On 05-Apr-01 John Baldwin wrote:
 
 On 05-Apr-01 Garrett Wollman wrote:
 On Thu, 05 Apr 2001 01:39:35 -0500, Richard Todd
 [EMAIL PROTECTED] said:
 
 If I'm reading this backtrace right, the thread handling the sound
 hardware called selwakeup() (frame #19).  This called pfind() (frame
 #18), which tries to lock allproc.
 
 selwakeup() shouldn't need to call pfind().  Because the process table
 is in type-stable memory, it should be sufficient to keep a reference
 to the caller's proc structure and check to see whether its pid is the
 same one as in the selinfo.  The locking that selwakeup() already
 needs to do should be sufficient to avoid a race.
 
 (In 4.4BSD, process structures were not type-stable so this technique
 could not have been used.)
 
 There are probably several other places that pfind is called that this check
 should also be adequate for as well.  The ones in syscons for example.

As a safety check we should probably zero the pid right before zfree()'ing a
proc in wait() however, so that a stale pointer to a free'd process doesn't
have a valid pid if we do this.

 -GAWollman

-- 

John Baldwin [EMAIL PROTECTED] -- http://www.FreeBSD.org/~jhb/
PGP Key: http://www.baldwin.cx/~john/pgpkey.asc
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: selwakeup()

2001-04-05 Thread Garrett Wollman

On Thu, 05 Apr 2001 14:41:29 -0700 (PDT), John Baldwin [EMAIL PROTECTED] said:

 As a safety check we should probably zero the pid right before zfree()'ing a
 proc in wait() however, so that a stale pointer to a free'd process doesn't
 have a valid pid if we do this.

Should not be necessary.  Here is the logic:

p = sip-si_p;
mtx_lock_spin(sched_lock);
if (p-p_stat != SZOMB || p-p_pid != sip-si_pid) {
/* oops */
mtx_lock_spin(sched_lock);
return;
}

sip-si_pid = 0;
sip-si_p = 0;
if (p-p_wchan == (caddr_t)selwait) {
/* ... */


If `p' is a pointer to a freed process, then p-p_stat is guaranteed
to be SZOMB -- the only code path which can free a process struct is
wrapped inside `if (p-p_stat == SZOMB)'.  (See kern_exit.c:exit1().)
If `p' is a pointer to an active process, and it's the wrong pid, then
we don't wake it up.  Otherwise, we wake it up.  (`p' might still be
the wrong process, if pid space wrapped around, but the current code
doesn't deal with that condition, either, nor should it.)

-GAWollman


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: selwakeup()

2001-04-05 Thread John Baldwin


On 05-Apr-01 Garrett Wollman wrote:
 On Thu, 05 Apr 2001 14:41:29 -0700 (PDT), John Baldwin [EMAIL PROTECTED]
 said:
 
 As a safety check we should probably zero the pid right before zfree()'ing a
 proc in wait() however, so that a stale pointer to a free'd process doesn't
 have a valid pid if we do this.
 
 Should not be necessary.  Here is the logic:

Ah, forgot about the p_stat check.

-- 

John Baldwin [EMAIL PROTECTED] -- http://www.FreeBSD.org/~jhb/
PGP Key: http://www.baldwin.cx/~john/pgpkey.asc
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message