Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 20 Feb 2004 10:38:30 +0300
From:      Mike Makonnen <mtm@identd.net>
To:        Tim Robbins <tjr@FreeBSD.org>
Cc:        freebsd-threads@FreeBSD.org
Subject:   Re: libthr patch
Message-ID:  <20040220073830.GB1089@mobile.acs-et.com>
In-Reply-To: <20040219134733.GA38023@cat.robbins.dropbear.id.au>
References:  <20040219062850.GC1074@mobile.acs-et.com> <20040219134733.GA38023@cat.robbins.dropbear.id.au>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Feb 20, 2004 at 12:47:33AM +1100, Tim Robbins wrote:
> On Thu, Feb 19, 2004 at 09:28:50AM +0300, Mike Makonnen wrote:
> 
> Make sure you fix thr_wake() to check that uap->id is valid before you
> commit this patch. Something like this would be safer but slower:

Hmm, you're right. I should've thought of that. Thanks for catching it.

> 
> 	struct thread *td1;
> 
> 	PROC_LOCK(td->td_proc);
> 	FOREACH_THREAD_IN_PROC(td->td_proc, td1)
> 		if (td1 == (struct thread *)uap->id)
> 			break;
> 	if (td1 == NULL) {
> 		PROC_UNLOCK(td->td_proc);
> 		return (ESRCH);
> 	}
> 	td1->td_lthrflags |= LTF_THRWAKEUP;
> 	wakeup_one(td1);
> 	PROC_UNLOCK(td->td_proc);
> 	return (0);
> 
> (I'm not sure that it's safe to call wakeup_one() on a thread pointer
> that isn't curthread without holding the proc lock, so I changed that too.)

Sorry, I don't follow. How can a thread call wakeup_one() on itself (if it's
supposed to be asleep)?

A cursory look through the call path doesn't show anything that needs a
proc lock. There is access to p->p_state but either sched_lock or proc lock
is sufficient for that (and the accessing function does hold sched_lock).

Cheers.
-- 
Mike Makonnen  | GPG-KEY: http://www.identd.net/~mtm/mtm.asc
mtm@identd.net | Fingerprint: AC7B 5672 2D11 F4D0 EBF8  5279 5359 2B82 7CD4 1F55
mtm@FreeBSD.Org| FreeBSD - Unleash the Daemon !



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040220073830.GB1089>