From owner-freebsd-threads@FreeBSD.ORG  Fri Feb 20 00:25:25 2004
Return-Path: <owner-freebsd-threads@FreeBSD.ORG>
Delivered-To: freebsd-threads@freebsd.org
Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP id 2984216A4CE
	for <freebsd-threads@FreeBSD.org>;
	Fri, 20 Feb 2004 00:25:25 -0800 (PST)
Received: from smtp02.syd.iprimus.net.au (smtp02.syd.iprimus.net.au
	[210.50.76.52])	by mx1.FreeBSD.org (Postfix) with ESMTP id EECB743D1D
	for <freebsd-threads@FreeBSD.org>;
	Fri, 20 Feb 2004 00:25:24 -0800 (PST)
	(envelope-from tim@robbins.dropbear.id.au)
Received: from robbins.dropbear.id.au (210.50.203.147) by
	smtp02.syd.iprimus.net.au (7.0.024)
	id 402CF870001D4185; Fri, 20 Feb 2004 19:25:21 +1100
Received: by robbins.dropbear.id.au (Postfix, from userid 1000)
	id 8469A41BF; Fri, 20 Feb 2004 19:25:17 +1100 (EST)
Date: Fri, 20 Feb 2004 19:25:17 +1100
From: Tim Robbins <tjr@freebsd.org>
To: Mike Makonnen <mtm@identd.net>
Message-ID: <20040220082517.GA42604@cat.robbins.dropbear.id.au>
References: <20040219062850.GC1074@mobile.acs-et.com>
	<20040219134733.GA38023@cat.robbins.dropbear.id.au>
	<20040220073830.GB1089@mobile.acs-et.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20040220073830.GB1089@mobile.acs-et.com>
User-Agent: Mutt/1.4.1i
cc: freebsd-threads@FreeBSD.org
Subject: Re: libthr patch
X-BeenThere: freebsd-threads@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
List-Id: Threading on FreeBSD <freebsd-threads.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-threads>,
	<mailto:freebsd-threads-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-threads>
List-Post: <mailto:freebsd-threads@freebsd.org>
List-Help: <mailto:freebsd-threads-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-threads>,
	<mailto:freebsd-threads-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 20 Feb 2004 08:25:25 -0000

On Fri, Feb 20, 2004 at 10:38:30AM +0300, Mike Makonnen wrote:

> 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).

What I mean is: I don't know whether you can be sure that the thread hasn't
already been freed between the time unlock the proc and when you call
wakeup_one(). I assumed the list of threads associated with a process was
protected by the proc lock.


Tim