svn commit: r263704 - head/sys/kern

[Warner Losh]

On Mar 24, 2014, at 9:28 PM, Mateusz Guzik <mjg at FreeBSD.org> wrote:

> Author: mjg
> Date: Tue Mar 25 03:28:58 2014
> New Revision: 263704
> URL: http://svnweb.freebsd.org/changeset/base/263704
> 
> Log:
>  Make /dev/devctl mpsafe.
> 
>  MFC after:	1 week
> 
> Modified:
>  head/sys/kern/subr_bus.c
> 
> Modified: head/sys/kern/subr_bus.c
> ==============================================================================
> --- head/sys/kern/subr_bus.c	Tue Mar 25 03:25:30 2014	(r263703)
> +++ head/sys/kern/subr_bus.c	Tue Mar 25 03:28:58 2014	(r263704)
> @@ -358,15 +358,16 @@ device_sysctl_fini(device_t dev)
> /* Deprecated way to adjust queue length */
> static int sysctl_devctl_disable(SYSCTL_HANDLER_ARGS);
> /* XXX Need to support old-style tunable hw.bus.devctl_disable" */
> -SYSCTL_PROC(_hw_bus, OID_AUTO, devctl_disable, CTLTYPE_INT | CTLFLAG_RW, NULL,
> -    0, sysctl_devctl_disable, "I", "devctl disable -- deprecated");
> +SYSCTL_PROC(_hw_bus, OID_AUTO, devctl_disable, CTLTYPE_INT | CTLFLAG_RW |
> +    CTLFLAG_MPSAFE, NULL, 0, sysctl_devctl_disable, "I",
> +    "devctl disable -- deprecated”);

This can likely be deleted now...

> #define DEVCTL_DEFAULT_QUEUE_LEN 1000
> static int sysctl_devctl_queue(SYSCTL_HANDLER_ARGS);
> static int devctl_queue_length = DEVCTL_DEFAULT_QUEUE_LEN;
> TUNABLE_INT("hw.bus.devctl_queue", &devctl_queue_length);
> -SYSCTL_PROC(_hw_bus, OID_AUTO, devctl_queue, CTLTYPE_INT | CTLFLAG_RW, NULL,
> -    0, sysctl_devctl_queue, "I", "devctl queue length");
> +SYSCTL_PROC(_hw_bus, OID_AUTO, devctl_queue, CTLTYPE_INT | CTLFLAG_RW |
> +    CTLFLAG_MPSAFE, NULL, 0, sysctl_devctl_queue, "I", "devctl queue length");
> 
> static d_open_t		devopen;
> static d_close_t	devclose;
> @@ -376,7 +377,6 @@ static d_poll_t		devpoll;
> 
> static struct cdevsw dev_cdevsw = {
> 	.d_version =	D_VERSION,
> -	.d_flags =	D_NEEDGIANT,
> 	.d_open =	devopen,
> 	.d_close =	devclose,
> 	.d_read =	devread,
> @@ -420,23 +420,31 @@ devinit(void)
> static int
> devopen(struct cdev *dev, int oflags, int devtype, struct thread *td)
> {
> +
> 	if (devsoftc.inuse)
> 		return (EBUSY);

Why not delete these two lines? Since this isn’t a performance critical part of the code,
it is clearer and safer to just test inuse inside the locked section.

> +	mtx_lock(&devsoftc.mtx);
> +	if (devsoftc.inuse) {
> +		mtx_unlock(&devsoftc.mtx);
> +		return (EBUSY);
> +	}
> 	/* move to init */
> 	devsoftc.inuse = 1;
> 	devsoftc.nonblock = 0;
> 	devsoftc.async_proc = NULL;
> +	mtx_unlock(&devsoftc.mtx);
> 	return (0);
> }
> 
> static int
> devclose(struct cdev *dev, int fflag, int devtype, struct thread *td)
> {
> -	devsoftc.inuse = 0;
> +
> 	mtx_lock(&devsoftc.mtx);
> +	devsoftc.inuse = 0;
> +	devsoftc.async_proc = NULL;
> 	cv_broadcast(&devsoftc.cv);
> 	mtx_unlock(&devsoftc.mtx);
> -	devsoftc.async_proc = NULL;
> 	return (0);
> }