Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 15 Jan 2015 05:31:09 +0200
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Hans Petter Selasky <hselasky@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r277199 - in head/sys: fs/devfs kern
Message-ID:  <20150115033109.GM42409@kib.kiev.ua>
In-Reply-To: <201501142207.t0EM7Dfn041543@svn.freebsd.org>
References:  <201501142207.t0EM7Dfn041543@svn.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150115033109.GM42409>