From owner-freebsd-hackers@FreeBSD.ORG Sun Mar 4 14:11:38 2007 Return-Path: X-Original-To: hackers@freebsd.org Delivered-To: freebsd-hackers@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 6FE0616A400 for ; Sun, 4 Mar 2007 14:11:38 +0000 (UTC) (envelope-from xdivac02@stud.fit.vutbr.cz) Received: from eva.fit.vutbr.cz (eva.fit.vutbr.cz [147.229.176.14]) by mx1.freebsd.org (Postfix) with ESMTP id 0AE6813C47E for ; Sun, 4 Mar 2007 14:11:37 +0000 (UTC) (envelope-from xdivac02@stud.fit.vutbr.cz) Received: from eva.fit.vutbr.cz (localhost [127.0.0.1]) by eva.fit.vutbr.cz (envelope-from xdivac02@eva.fit.vutbr.cz) (8.13.8/8.13.7) with ESMTP id l24EBasZ006934 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Sun, 4 Mar 2007 15:11:36 +0100 (CET) Received: (from xdivac02@localhost) by eva.fit.vutbr.cz (8.13.8/8.13.3/Submit) id l24EBaIj006932 for hackers@freebsd.org; Sun, 4 Mar 2007 15:11:36 +0100 (CET) Date: Sun, 4 Mar 2007 15:11:36 +0100 From: Divacky Roman To: hackers@freebsd.org Message-ID: <20070304141136.GA4935@stud.fit.vutbr.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4.2.2i X-Scanned-By: MIMEDefang 2.57 on 147.229.176.14 Cc: Subject: investigation of Giant usage in kernel X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Mar 2007 14:11:38 -0000 hi I looked at where Giant is held in the kernel and I found these interesting things: 1) in fs/fifofs/fifo_vnops.c we lock Giant when calling sorecieve()/sosend() this is a bandaid for fixing a race that doesnt have to exist anymore. ie. it needs some testing and can be remvoed 2) in geom/geom_dev.c we lock Giant for: 2a) calling make_dev() which seems unecessary because make_dev protects itself with devmtx 2b) setting a flag in cdev which I think is unecessary as well 3) in kern/kern_descrip.c we lock Giant for the whole life of flock() I dont see a need for this protection becuase 3a) access to fp is protected with FILE_LOCK()ing 3b) otherwise it operates on local variables 3c) it also calls VOP_* functions but those dont require Giant, right? 4) in kern/kern_lockf.c we lock Giant for the whole life of lf_advlock() I dont think this is necessary because 4a) it operates on local variables 4b) it calls various lf_*lock() functions which seems to either protect themselves or not needed protection at all 5) in kern/kern_time.c we lock Giant to protect 5a) tc_setclock() call - I dont know if this is necessary or not 5b) lease_updatetime call which is a function pointer that seems to be only assigned at one place (line 119 in kern_time.c) to a NOP function. can this be removed? 6) in vm/device_pager.c we lock Giant to (also) protect cdevsw->d_mmap but the device mmap is either MPSAFE or assigned to giant_mmap (a wrapper that locks GIant and calls the original d_mmap) so this seems unecessary. These are just my impressions on the code and I'd like to hear some comments if its correct and if its I'd like to see some fixes. thnx roman