From owner-svn-src-head@FreeBSD.ORG Thu Feb 27 23:17:45 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 6F163165; Thu, 27 Feb 2014 23:17:45 +0000 (UTC) Received: from mail-qc0-x22f.google.com (mail-qc0-x22f.google.com [IPv6:2607:f8b0:400d:c01::22f]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id D655B123A; Thu, 27 Feb 2014 23:17:44 +0000 (UTC) Received: by mail-qc0-f175.google.com with SMTP id e16so406680qcx.6 for ; Thu, 27 Feb 2014 15:17:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=gvukGNDMRBDZ0Hb+OwYe6H/PA8QL+rKV5VowF+cC3SM=; b=sKshIgDRlZsnaAT/UqScF4gkljilY0pQHW3XQ7CUCgQ0u+Jha3svX3yTX3zy78Xa2k p8Le81G+Z3MqVVpCPHp3w+s5syUcGKUq11NSO9UbxauPZYLLtLaVydjA2qUJbCZTdrVr uYm3WymWId30XW/0Mxj1qypRrpeOaIbkfpepCCm4LllKfS6L9jRiCc8U6yUR+b+vrUr/ mgEDRFIrPGfKCMvcC6gObgD5IrcTzZyhgIT7/KnPXz9mARiVMPes7Z3tbDoeEiFt/ZDS iS67sQNZEKvOHOWTIEYMcy1GfPSsAmEft8aKPaM5OS8Ck8KGmXhhA/2MESxutnpFKpuD mFSA== X-Received: by 10.140.26.240 with SMTP id 103mr11297334qgv.92.1393543063942; Thu, 27 Feb 2014 15:17:43 -0800 (PST) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id u20sm6728064qge.2.2014.02.27.15.17.42 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 27 Feb 2014 15:17:43 -0800 (PST) Date: Fri, 28 Feb 2014 00:17:38 +0100 From: Mateusz Guzik To: John Baldwin Subject: Re: svn commit: r262309 - head/sys/kern Message-ID: <20140227231738.GA24050@dft-labs.eu> References: <201402212229.s1LMT9BF093587@svn.freebsd.org> <201402261529.31942.jhb@freebsd.org> <20140226212200.GB329@dft-labs.eu> <201402270906.31359.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <201402270906.31359.jhb@freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Feb 2014 23:17:45 -0000 On Thu, Feb 27, 2014 at 09:06:31AM -0500, John Baldwin wrote: > On Wednesday, February 26, 2014 4:22:00 pm Mateusz Guzik wrote: > > On Wed, Feb 26, 2014 at 03:29:31PM -0500, John Baldwin wrote: > > > On Wednesday, February 26, 2014 2:23:48 pm Mateusz Guzik wrote: > > > > Other thing is that with that change in place we can get rid of > > > > XLOCK/XUNLOCK around fdfree in fdescfree. > > > > > > I would rather remove the fd_refcnt checks, or do them differently (not > > > in the loop). Right now a reader might be confused to think that > > > fd_refcnt can change within the loop when it cannot and then worry about > > > unhandled races that don't exist (i.e. if fd_refcnt can change within > > > the loop, what prevents the individual file objects from being freed out > > > from under the loop?) > > > > > > > But it can change. > > > > kern_proc_filedesc_out calls export_fd_to_sb which drops the lock for > > each fp and sysctl_kern_proc_ofiledesc drops the lock when dealing with > > vnodes. > > > > As far as I can say all this is safe - either data is refed (vref on a > > vnode) or the lock is still held while the data is being read, so by the > > time fp can be freed it is no longer used. > > Ugh, ok. Then the change is fine as-is, but I think we have to leave > the locking in place around fdfree() still as a result. > I don't see why. refcnt cannot drop as long as something holds fdp lock. 1) So let's say kern_proc_filedesc_out grabs the lock, refcnt is 0. No files are inspected and the loop is terminated. 2) So let's say refcnt is 1 and fp is being read. Lock is released only when the function is done with fp. Then fdescfree drops refcnt to 0 and proceeds to free fps. And we are back to 1). IOW I don't think locking around fdfree is of any use right now, although I don't feel strongly about removing it. -- Mateusz Guzik