From owner-cvs-src@FreeBSD.ORG Mon Mar 5 09:32:28 2007 Return-Path: X-Original-To: cvs-src@FreeBSD.org Delivered-To: cvs-src@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 8364A16A400; Mon, 5 Mar 2007 09:32:28 +0000 (UTC) (envelope-from mux@freebsd.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id 706E613C481; Mon, 5 Mar 2007 09:32:28 +0000 (UTC) (envelope-from mux@freebsd.org) Received: by elvis.mu.org (Postfix, from userid 1920) id 7C7A41A4D90; Mon, 5 Mar 2007 01:13:29 -0800 (PST) Date: Mon, 5 Mar 2007 10:13:29 +0100 From: Maxime Henrion To: Greg Lehey Message-ID: <20070305091329.GA65356@elvis.mu.org> References: <200703050559.l255xnxK035158@repoman.freebsd.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200703050559.l255xnxK035158@repoman.freebsd.org> User-Agent: Mutt/1.4.2.2i Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/share/man/man9 sleep.9 X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Mar 2007 09:32:28 -0000 Greg Lehey wrote: > grog 2007-03-05 05:59:49 UTC > > FreeBSD src repository > > Modified files: > share/man/man9 sleep.9 > Log: > Another typo. > > Spotted by: ru > Another pointy hat to: grog > > Revision Changes Path > 1.58 +1 -1 src/share/man/man9/sleep.9 I've been reading the latest version of this manpage with your modifications, and still find the information you have added in it largely misleading and/or wrong. I'm particularly worried by this paragraph : Due to the way it works, the wakeup_one() function requires that only related threads sleep on a specific chan address. It is the programmer's responsibility to choose a unique chan value. The older wakeup() func- tion did not require this, though it was never good practice for threads to share a chan value. When converting from wakeup() to wakeup_one(), pay particular attention to ensure that no other threads wait on the same chan. You're making it look as if sharing the wait channel when using wakeup_one() is more of a problem than when using wakeup(). This isn't true; both usages are completely wrong. What is different is that when in the wakeup_one() case, the symptoms are likely to be more apparent and more annoying because chances are that it will lead to deadlocks pretty quickly. This does not make sharing wait channels "more" OK within the context of wakeup(). Also, this particular bit : "The older wakeup() function did not require this, [...]" looks entirely wrong. You are somehow mitigating it by saying that it was never a good practice to share wait channels afterwards, but this is still untrue. Sharing wait channels isn't just "not a good practice", it is entirely incorrect. As far as I can tell, the wakeup() function always required separate wait channels, it's just that the effect of the sharing wasn't necessarily visible, if every process/thread checks that the condition it was waiting on is indeed met when it is woken up (and that it goes back to sleep if it isn't). I think these changes should be backed out. As a side note, we really need to migrate from the msleep()/wakeup() API to using CVs. It would make those kind of confusions impossible. Cheers, Maxime