From owner-freebsd-hackers@FreeBSD.ORG Mon Jul 16 12:43:53 2007 Return-Path: X-Original-To: freebsd-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 5F8B116A40A; Mon, 16 Jul 2007 12:43:53 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail6.speedfactory.net [66.23.216.219]) by mx1.freebsd.org (Postfix) with ESMTP id 91F9E13C4B9; Mon, 16 Jul 2007 12:43:52 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.7b8) with ESMTP id 196926955 for multiple; Mon, 16 Jul 2007 08:36:32 -0400 Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.8/8.13.8) with ESMTP id l6GCSBkF031595; Mon, 16 Jul 2007 08:28:14 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: freebsd-hackers@freebsd.org Date: Mon, 16 Jul 2007 07:41:41 -0400 User-Agent: KMail/1.9.6 References: <20070614075439.GA71601@freebsd.org> <20070614075830.GA71887@freebsd.org> In-Reply-To: <20070614075830.GA71887@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200707160741.44094.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Mon, 16 Jul 2007 08:28:15 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/3680/Mon Jul 16 01:49:06 2007 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx X-Server: High Performance Mail Server - http://surgemail.com r=1653887525 Cc: Roman Divacky Subject: Re: [PATCH]: acct_process() locking and exit1() 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: Mon, 16 Jul 2007 12:43:53 -0000 On Thursday 14 June 2007 03:58:30 am Roman Divacky wrote: > On Thu, Jun 14, 2007 at 09:54:39AM +0200, Roman Divacky wrote: > > hi > > > > currently in exit1() we call acct_process() with Giant held. > > I looked at the code and I think this is because of tty need > > for Giant locking. Because of this we have to release process limit > > in a separate PROC_LOCK()/UNLOCK() block. > > > > my just-to-look-at patch does this > > > > 1) moves the acct_process() in exit1() out of Giant locked block (upward) > > 2) acquires Giant in the acct_process() around tty handling > > 3) moves the p->p_limit to a different PROC_LOCK/UNLOCK block (upward) > > > > the result is saving 2 proc mtx operations in exit1() > > > > can you look at it and tell me if its correct or wrong or something like that? > > > > if it's correct is it worth committing? saving two mtx operations is a nice thing :) > > erm... two bugs ;) "saving two mtx operations..." in the typical case of accounting > NOT being enabled. > > and forgot to show the patch ;) it's here > > www.vlakno.cz/~rdivacky/kern_acct.patch 1) It would be nicer if lim_free() wasn't so far away from the rest of the limit freeing code (including the comment). Also, missing a blank line before the comment now. 2) acct_process() saves resource usage information, so the later it is in exit1() the more accurate it is going to be, so it's probably best to leave it as it is unless you observe a measurable change in a benchmark with this patch. -- John Baldwin