From owner-svn-src-all@FreeBSD.ORG Thu Jan 15 03:31:16 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 08E13B8; Thu, 15 Jan 2015 03:31:16 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 8AB55D15; Thu, 15 Jan 2015 03:31:15 +0000 (UTC) Received: from tom.home (kostik@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id t0F3V99s023713 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 15 Jan 2015 05:31:09 +0200 (EET) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua t0F3V99s023713 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id t0F3V9Zt023711; Thu, 15 Jan 2015 05:31:09 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Thu, 15 Jan 2015 05:31:09 +0200 From: Konstantin Belousov To: Hans Petter Selasky Subject: Re: svn commit: r277199 - in head/sys: fs/devfs kern Message-ID: <20150115033109.GM42409@kib.kiev.ua> References: <201501142207.t0EM7Dfn041543@svn.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201501142207.t0EM7Dfn041543@svn.freebsd.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Jan 2015 03:31:16 -0000 On Wed, Jan 14, 2015 at 10:07:13PM +0000, Hans Petter Selasky wrote: > Author: hselasky > Date: Wed Jan 14 22:07:13 2015 > New Revision: 277199 > URL: https://svnweb.freebsd.org/changeset/base/277199 > > Log: > Avoid race with "dev_rel()" when using the recently added > "delist_dev()" function. Make sure the character device structure > doesn't go away until the end of the "destroy_dev()" function due to > concurrently running cleanup code inside "devfs_populate()". > > MFC after: 1 week > Reported by: dchagin@ > > Modified: > head/sys/fs/devfs/devfs_devs.c > head/sys/kern/kern_conf.c > > Modified: head/sys/fs/devfs/devfs_devs.c > ============================================================================== > --- head/sys/fs/devfs/devfs_devs.c Wed Jan 14 22:05:57 2015 (r277198) > +++ head/sys/fs/devfs/devfs_devs.c Wed Jan 14 22:07:13 2015 (r277199) > @@ -137,6 +137,12 @@ devfs_alloc(int flags) > vfs_timestamp(&ts); > cdev->si_atime = cdev->si_mtime = cdev->si_ctime = ts; > cdev->si_cred = NULL; > + /* > + * Avoid race with dev_rel() by setting the initial > + * reference count to 1. This last reference is taken > + * by the destroy_dev() function. > + */ > + cdev->si_refcount = 1; This is wrong. Not all devices are destroyed with destroy_dev(). dev_rel() must be allowed to clean up allocated device. That said, I do not understand what race you are trying to solve. Freeing of the accessible cdev memory cannot happen in parallel while dev_mtx is owned. Please do not commit (to devfs) without seeking for the review first. > > return (cdev); > } > > Modified: head/sys/kern/kern_conf.c > ============================================================================== > --- head/sys/kern/kern_conf.c Wed Jan 14 22:05:57 2015 (r277198) > +++ head/sys/kern/kern_conf.c Wed Jan 14 22:07:13 2015 (r277199) > @@ -1048,8 +1048,6 @@ destroy_devl(struct cdev *dev) > /* Remove name marking */ > dev->si_flags &= ~SI_NAMED; > > - dev->si_refcount++; /* Avoid race with dev_rel() */ > - > /* If we are a child, remove us from the parents list */ > if (dev->si_flags & SI_CHILD) { > LIST_REMOVE(dev, si_siblings);