From owner-cvs-src@FreeBSD.ORG Mon Nov 20 14:21:07 2006 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 3ED4416A532; Mon, 20 Nov 2006 14:21:07 +0000 (UTC) (envelope-from bde@zeta.org.au) Received: from mailout1.pacific.net.au (mailout1-3.pacific.net.au [61.8.2.210]) by mx1.FreeBSD.org (Postfix) with ESMTP id 25E4243D5D; Mon, 20 Nov 2006 14:20:41 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy1.pacific.net.au (mailproxy1.pacific.net.au [61.8.2.162]) by mailout1.pacific.net.au (Postfix) with ESMTP id 1026F5DFEAD; Tue, 21 Nov 2006 01:20:49 +1100 (EST) Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailproxy1.pacific.net.au (Postfix) with ESMTP id C00C58C04; Tue, 21 Nov 2006 01:20:47 +1100 (EST) Date: Tue, 21 Nov 2006 01:20:46 +1100 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Mohan Srinivasan In-Reply-To: <200611200414.kAK4EN1T032693@repoman.freebsd.org> Message-ID: <20061121010050.P25283@delplex.bde.org> References: <200611200414.kAK4EN1T032693@repoman.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/nfsclient nfs.h nfs_socket.c 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, 20 Nov 2006 14:21:07 -0000 On Mon, 20 Nov 2006, Mohan Srinivasan wrote: > mohans 2006-11-20 04:14:23 UTC > > FreeBSD src repository > > Modified files: > sys/nfsclient nfs.h nfs_socket.c > Log: > 1) Fix up locking in nfs_up() and nfs_down. > 2) Reduce the acquisitions of the Giant lock in the nfs_socket.c paths significantly. > - We don't need to acquire Giant before tsleeping on lbolt anymore, > since jhb specialcased lbolt handling in msleep. > - nfs_up() needs to acquire Giant only if printing the "server up" > message. Giant isn't required here either. tprintf() does its own locking as required. This locking happens to be Giant locking, and callers unfortunately have to be aware of this to avoid LORs. It may be necessary to acquire at a higher level, but fortunately, most calls to tprintf() are already at a high level, and since you've managed to push down the calls here, the calls are apparently already at a high level. Giant should never have been required for calling tprintf() but it became necessary when Giant was first implemented (since calls into the tty driver require Giant), and for many years all calls to it from file systems only worked because file systems were Giant locked. Then, some time after file systems became not Giant locked, the crashes caused by calling tprintf() without Giant were incorrectly fixed for a few days by acquiring Giant in many callers instead of only in tprintf(). Giant was never required for plain printf(). nfs_printf() was never needed since it only wraps plain printf() with Giant locking. > - nfs_timer() held Giant for the duration of the NFS timer processing, > just because the printing of the message in nfs_down() needed it > (and we acquire other locks in nfs_timer()). The acquisition of > Giant is moved down into nfs_down() now, reducing the time Giant is > held in that path. Like nfs_up(). Bruce