Date: Tue, 25 Mar 2014 08:03:13 -0600 From: Warner Losh <imp@bsdimp.com> To: Mateusz Guzik <mjg@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r263704 - head/sys/kern Message-ID: <F0A53D44-BC30-4A02-8DD0-2A1B8711F2C7@gmail.com> In-Reply-To: <201403250328.s2P3Swsl054558@svn.freebsd.org> References: <201403250328.s2P3Swsl054558@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mar 24, 2014, at 9:28 PM, Mateusz Guzik <mjg@FreeBSD.org> wrote: > Author: mjg > Date: Tue Mar 25 03:28:58 2014 > New Revision: 263704 > URL: http://svnweb.freebsd.org/changeset/base/263704 >=20 > Log: > Make /dev/devctl mpsafe. >=20 > MFC after: 1 week >=20 > Modified: > head/sys/kern/subr_bus.c >=20 > Modified: head/sys/kern/subr_bus.c > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D > --- 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=94); 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 =3D 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"); >=20 > static d_open_t devopen; > static d_close_t devclose; > @@ -376,7 +377,6 @@ static d_poll_t devpoll; >=20 > static struct cdevsw dev_cdevsw =3D { > .d_version =3D D_VERSION, > - .d_flags =3D D_NEEDGIANT, > .d_open =3D devopen, > .d_close =3D devclose, > .d_read =3D 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=92t 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 =3D 1; > devsoftc.nonblock =3D 0; > devsoftc.async_proc =3D NULL; > + mtx_unlock(&devsoftc.mtx); > return (0); > } >=20 > static int > devclose(struct cdev *dev, int fflag, int devtype, struct thread *td) > { > - devsoftc.inuse =3D 0; > + > mtx_lock(&devsoftc.mtx); > + devsoftc.inuse =3D 0; > + devsoftc.async_proc =3D NULL; > cv_broadcast(&devsoftc.cv); > mtx_unlock(&devsoftc.mtx); > - devsoftc.async_proc =3D NULL; > return (0); > }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F0A53D44-BC30-4A02-8DD0-2A1B8711F2C7>