From owner-freebsd-smp@FreeBSD.ORG Thu Nov 29 21:05:18 2007 Return-Path: Delivered-To: freebsd-smp@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id CDB1416A419 for ; Thu, 29 Nov 2007 21:05:18 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from nf-out-0910.google.com (nf-out-0910.google.com [64.233.182.190]) by mx1.freebsd.org (Postfix) with ESMTP id 3C4CF13C461 for ; Thu, 29 Nov 2007 21:05:17 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: by nf-out-0910.google.com with SMTP id b2so1969594nfb for ; Thu, 29 Nov 2007 13:05:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth; bh=5tPk+4Lx3gyDjwDZUHZWi+XX4elNgdWfcXiHPMjKaSY=; b=SEN6aD55rmvURq0rlEgrReleDuqXu87ta2G6nDBU2N6gk0Q8O3gH75XnnFN4mz7A+N22/O+NdWYYRwHGNc8aVtxXgSoeltn94ca0lQaupYxV+TmYCp0+lUjxD80WQpX4DxUnHJkANXrtlJlfCtBPu0RN2z5BIj63F5+uBtu/ukQ= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=received:message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth; b=jtMYLhYuB2WMM7Q7U/1TR6Vss4Bnu0fgUARTB1U/yDJvceCQ0qEqBuKqAvhx++Q27oGwHJxScYv6XDg/5AUxc7AnB524rFmyc294DVkRWB3/HZEWBkZkzBLfZ3sAGGAQmkZ/KKM3jd7/TPrDc7xDCQQUkWsyVYUYI4fU/MPuMWA= Received: by 10.86.71.1 with SMTP id t1mr6447312fga.1196368760439; Thu, 29 Nov 2007 12:39:20 -0800 (PST) Received: by 10.86.28.19 with HTTP; Thu, 29 Nov 2007 12:39:20 -0800 (PST) Message-ID: <3bbf2fe10711291239p553af196ma406fc67fb1c350d@mail.gmail.com> Date: Thu, 29 Nov 2007 21:39:20 +0100 From: "Attilio Rao" Sender: asmrookie@gmail.com To: "Robert Watson" In-Reply-To: <3bbf2fe10711231635i72df8babucedd1e5bdef7175d@mail.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20071121222319.GX44563@elvis.mu.org> <200711221641.02484.max@love2party.net> <3bbf2fe10711220753u435ff4cbxa94d5b682292b970@mail.gmail.com> <200711221726.27108.max@love2party.net> <20071123082339.GN44563@elvis.mu.org> <47469328.8020404@freebsd.org> <20071123092415.GP44563@elvis.mu.org> <4746F858.4070301@freebsd.org> <20071123235346.E14018@fledge.watson.org> <3bbf2fe10711231635i72df8babucedd1e5bdef7175d@mail.gmail.com> X-Google-Sender-Auth: 627131c92186db98 Cc: freebsd-smp@freebsd.org, Peter Holms , Jeff Roberson , Alfred Perlstein , freebsd-arch@freebsd.org, Stephan Uphoff , Max Laier Subject: Re: rwlocks, correctness over speed. X-BeenThere: freebsd-smp@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD SMP implementation group List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Nov 2007 21:05:19 -0000 2007/11/24, Attilio Rao : > 2007/11/24, Robert Watson : > > > > On Fri, 23 Nov 2007, Stephan Uphoff wrote: > > > > >>> I talked with Attilio about that on IRC. Most common cases of writer > > >>> starvation (but not all) could be solved by keeping a per thread count of > > >>> shared acquired rwlocks. If a rwlock is currently locked as shared/read > > >>> AND a thread is blocked on it to lock it exclusively/write - then new > > >>> shared/read locks will only be granted to thread that already has a shared > > >>> lock. (per thread shared counter is non zero) > > >>> > > >>> To be honest I am a bit twitchy about a lock without priority propagation > > >>> - especially since in FreeBSD threads run with user priority in kernel > > >>> space and can get preempted. > > >> > > >> That's an interesting hack, I guess it could be done. > > >> > > >> I would still like to disallow recursion. > > >> > > > Oh - I am all for disallowing recursion. In my opinion the only valid place > > > for a thread to acquire the same lock multiple times is inside a transaction > > > system with full deadlock detection. The question is if we can do that this > > > late in the game? Maybe we could make non recursive the default and add a > > > call rw_allow_recurse or rw_init_recurse to allow recursion on a lock if we > > > can't get away with the straight out removal of the option? (Or less > > > desirable - keep the current default and add functions to disallow > > > recursion) > > > > While I'm no great fan of recursion, the reality is that many of our kernel > > subsystems are not yet ready to disallow recursion on locks. Take a look at > > the cases where we explicitly enable recursive acquisition for mutexes--in > > practice, most network stack mutexes are recursive due to the recursive > > calling in the network stack. While someday I'd like to think we'll be able > > to eliminate some of that, but it won't be soon since it requires significant > > reworking of very complicated code. The current model in which recursion is > > explicitly enabled only where still required seems to work pretty well for the > > existing code, although it's hard to say yet in the code I've looked at > > whether read recursion would be required--the situations I have in mind would > > require purely write recursion. There's one case in the UNIX domain socket > > code where we do a locked test and conditional lock/unlock with an rwlock for > > exclusive locking because recursion isn't currently supported, and that's not > > a usage I'd like to encourage more of. > > I think however that Stephan is just referring to the readers > recursion (as we are doing) that if present should be just in few > points. > If we want todo a radical switch of this genre this is the right > moment, just before rwlock became too widespread. > > I have a patch against witness which would check for readers recursion > and panic, I will post in the night or tomorrow. So, this patch checks for recursive readers and will panic in case: http://people.freebsd.org/~attilio/recursion_checks.diff (I preferred to do in this way, but you can check for perforce commit number 129792 for an alternative way to achieve the same). What I'm looking for now is for people which can apply this patch, enable WITNESS with all the relevant options on and try to see if there are recursive readers in rwlocks somewhere (hopefully not after the pfil(9) switching to rmlock that mlaier did recently). Attilio -- Peace can only be achieved by understanding - A. Einstein