From owner-freebsd-arch@FreeBSD.ORG Mon May 21 04:37:31 2007 Return-Path: X-Original-To: arch@freebsd.org Delivered-To: freebsd-arch@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 7A03116A400 for ; Mon, 21 May 2007 04:37:31 +0000 (UTC) (envelope-from jroberson@chesapeake.net) Received: from webaccess-cl.virtdom.com (webaccess-cl.virtdom.com [216.240.101.25]) by mx1.freebsd.org (Postfix) with ESMTP id 2BC6B13C44C for ; Mon, 21 May 2007 04:37:31 +0000 (UTC) (envelope-from jroberson@chesapeake.net) Received: from [192.168.1.101] (c-71-231-138-78.hsd1.or.comcast.net [71.231.138.78]) (authenticated bits=0) by webaccess-cl.virtdom.com (8.13.6/8.13.6) with ESMTP id l4L4bSkH055454 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES256-SHA bits=256 verify=NO); Mon, 21 May 2007 00:37:29 -0400 (EDT) (envelope-from jroberson@chesapeake.net) Date: Sun, 20 May 2007 21:37:25 -0700 (PDT) From: Jeff Roberson X-X-Sender: jroberson@10.0.0.1 To: Bruce Evans In-Reply-To: <20070521113648.F86217@besplex.bde.org> Message-ID: <20070520213132.K632@10.0.0.1> References: <20070520155103.K632@10.0.0.1> <20070521113648.F86217@besplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: arch@freebsd.org Subject: Re: sched_lock && thread_lock() X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 May 2007 04:37:31 -0000 On Mon, 21 May 2007, Bruce Evans wrote: > On Sun, 20 May 2007, Jeff Roberson wrote: > >> Attilio and I have been working on addressing the increasing problem of >> sched_lock contention on -CURRENT. Attilio has been addressing the parts >> of the kernel which do not need to fall under the scheduler lock and moving >> them into seperate locks. For example, the ldt/gdt lock and clock lock >> which were committed earlier. Also, using atomics for the vmcnt structure. > > Using atomics in the vmmeter struct is mostly just a pessimization and > and obfuscation, since locks are still needed for accesses to more > than one variable at a time. For these cases, locks are needed for You are right, there are some cases which this pessimized. I wanted to make sure the cnt members that previously were protected by the sched_lock were still correct. However, I overlooked some of these which were accessed many at a time. What should happen is we should find out if any locks do protect the remaining members and if so, don't use VMCNT*, but mark the header describing how they are protected. > correctness, and are also probably more efficient if there are > 2 > accesses. Then lock poisoning requires the same locks to be held for > updates of the variables (to prevent variables changing while you are > using them), and atomic updates of single variables are a pessimization > too. The VMCNT*() ineterface encorages this pessimization and has other > problems (messes with volatile, and not actually doing atomic accesses > for VMCNT_GET()). Some of the cases where locks aren't needed (mainly > for userland-only statistics) were already handled better by > PCPU_LAZY_INC(). Unfortunately, the method used in PCPU_LAZY_INC() > doesn't work for variables that the kernel wants to access as globals. Depending on the architecture PCPU may be more expensive than atomics so we decied not to use it all over. I'll revisit the places with multiple accesses with Attilio and clean this up. Thanks, Jeff > > Example of code with multiple accesses: > > % /* > % * Return the number of pages we need to free-up or cache > % * A positive number indicates that we do not have enough free pages. > % */ > % % static __inline % int > % vm_paging_target(void) > % { > % return ( > % (VMCNT_GET(free_target) + VMCNT_GET(cache_min)) - % > (VMCNT_GET(free_count) + VMCNT_GET(cache_count)) > % ); > % } > > Without holding a lock throughout this, this returns a garbage value. > Without holding a lock throughout this and the actions taken depending > on the return value, garbage actions may be taken. Values that are > only slightly wrong might work OK, but this is not clear, and if it > does work then volatile variables and [non-]atomic accesses to the > variables to get it. > > Bruce >