From owner-freebsd-threads@FreeBSD.ORG Thu Feb 19 23:38:31 2004 Return-Path: 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 093B016A4CE; Thu, 19 Feb 2004 23:38:31 -0800 (PST) Received: from telecom.net.et (ns2.telecom.net.et [213.55.64.38]) by mx1.FreeBSD.org (Postfix) with ESMTP id E4AF643D2D; Thu, 19 Feb 2004 23:38:26 -0800 (PST) (envelope-from mtm@identd.net) Received: from [213.55.67.100] (HELO pool-151-200-10-97.res.east.verizon.net) by telecom.net.et (CommuniGate Pro SMTP 3.4.8) with ESMTP-TLS id 37116927; Fri, 20 Feb 2004 10:32:43 +0300 Received: from mobile.acs-et.com (localhost [127.0.0.1]) ESMTP id i1K7cYCN001477; Fri, 20 Feb 2004 10:38:36 +0300 (EAT) (envelope-from mtm@mobile.acs-et.com) Received: (from mtm@localhost) by mobile.acs-et.com (8.12.10/8.12.10/Submit) id i1K7cX5H001476; Fri, 20 Feb 2004 10:38:33 +0300 (EAT) (envelope-from mtm) Date: Fri, 20 Feb 2004 10:38:30 +0300 From: Mike Makonnen To: Tim Robbins Message-ID: <20040220073830.GB1089@mobile.acs-et.com> References: <20040219062850.GC1074@mobile.acs-et.com> <20040219134733.GA38023@cat.robbins.dropbear.id.au> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20040219134733.GA38023@cat.robbins.dropbear.id.au> User-Agent: Mutt/1.4.1i X-Operating-System: FreeBSD/5.2-CURRENT (i386) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Feb 2004 07:38:31 -0000 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 !