From owner-freebsd-current@FreeBSD.ORG Tue Dec 2 12:36:15 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id ECF1416A4CE; Tue, 2 Dec 2003 12:36:15 -0800 (PST) Received: from hak.cnd.mcgill.ca (hak.cnd.mcgill.ca [132.216.11.133]) by mx1.FreeBSD.org (Postfix) with ESMTP id 021F943F3F; Tue, 2 Dec 2003 12:36:14 -0800 (PST) (envelope-from mat@hak.cnd.mcgill.ca) Received: from hak.cnd.mcgill.ca (localhost [127.0.0.1]) by hak.cnd.mcgill.ca (8.12.9/8.12.8) with ESMTP id hB2KXU15062495; Tue, 2 Dec 2003 15:33:30 -0500 (EST) (envelope-from mat@hak.cnd.mcgill.ca) Received: (from mat@localhost) by hak.cnd.mcgill.ca (8.12.9/8.12.8/Submit) id hB2KXU1W062494; Tue, 2 Dec 2003 15:33:30 -0500 (EST) Date: Tue, 2 Dec 2003 15:33:30 -0500 From: Mathew Kanner To: Maxime Henrion Message-ID: <20031202203330.GF54011@cnd.mcgill.ca> References: <20031201142022.GK8404@elvis.mu.org> <20031201193837.GD49341@cnd.mcgill.ca> <20031201230259.GL8404@elvis.mu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20031201230259.GL8404@elvis.mu.org> User-Agent: Mutt/1.4.1i Organization: I speak for myself, operating in Montreal, CANADA X-Spam-Status: No, hits=1.2 required=5.0 tests=NO_EXPERIENCE autolearn=no version=2.60 X-Spam-Level: * X-Spam-Checker-Version: SpamAssassin 2.60 (1.212-2003-09-23-exp) on hak.cnd.mcgill.ca cc: Jesse Guardiani cc: freebsd-current@freebsd.org Subject: Re: 5.2-BETA dsp.c duplicate lock X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Dec 2003 20:36:16 -0000 On Dec 01, Maxime Henrion wrote: > Mathew Kanner wrote: > [patch ripped] > > > > Maxime, > > I think it would be better to isolate the changes (DUP_OK flag > > and lock creation) to just the channel code, no need to touch every > > driver. > > Yes, but to do this I'd need either to make the channel code use > mtx_init() directly, which would defeat the purpose of the USING_MUTEX > define, or to completely unifdef -U it. Since I had no idea if this code > was actually used or not, I went the safe way and just changed the > snd_mtxcreate() wrapper interface to accept mutex options. For what it's > worth, there is at least one direct invokation of mtx_init() in the sound > drivers, so it seems this define is actually already broken. Funny, I was thinking the exact same thing the other day. I'm %100 for this idea. > This needs to be sorted out before committing this patch if we need to, > but for now, I just wanted to see if using MTX_DUPOK solved the problem or > not. I believe that your patch should fix the problem. In general I see one of three strategies, 1) Your patch, 2) create a new snd_mtxcreate_chan for channels that sets the flags DUP_OK. 3) Fix locking to never hold duplicates. First glance suggests that would be contained in dsp.c, the ioctl handler is the real problem and seems inconsistent with itself in regards to locking. Ugh. > > > Also, if this is the right direction, we should back out the > > commit I did that re-ordered code that prevented duplicate channel > > locks (obviously it wasn't completen) channel. > > If the code is not supposed to acquire several channel locks at once, > then yes, it's not the right direction to go.As I said in my previous > mail, and that's why I wanted the advice of you sound guys, in that case > it's just a workaround. :-) I hear you. The fact is, currently there are no experienced sound people. Anyway, my order of preference is #2, #3, #1. Since #1 exists, I don't mind it being committed until we really understand what's going on. I will work on #3 tonight and maybe fallback to #2. Cheers, --Mat -- Applicants must also have extensive knowledge of UNIX, although they should have sufficiently good programming taste to not consider this an achievement. - MIT AI Lab job ad in the /Boston Globe/