svn commit: r278320 - in head: contrib/mdocml lib lib/libdevctl share/mk sys/dev/acpica sys/dev/pci sys/kern sys/sys usr.sbin usr.sbin/devctl

[John Baldwin]

On Thursday, February 26, 2015 08:28:33 PM Mateusz Guzik wrote:
> On Thu, Feb 26, 2015 at 07:46:38AM -0500, John Baldwin wrote:
> > On Thursday, February 26, 2015 12:40:06 AM Mateusz Guzik wrote:
> > > On Fri, Feb 06, 2015 at 04:09:02PM +0000, John Baldwin wrote:
> > > > Author: jhb
> > > > Date: Fri Feb  6 16:09:01 2015
> > > > New Revision: 278320
> > > > URL: https://svnweb.freebsd.org/changeset/base/278320
> > > > 
> > > > Log:
> > > >   Add a new device control utility for new-bus devices called devctl.
> > > >   This
> > > >   allows the user to request administrative changes to individual
> > > >   devices
> > > 
> > > [..]
> > > 
> > > > +static int
> > > > +devctl2_ioctl(struct cdev *cdev, u_long cmd, caddr_t data, int fflag,
> > > > +    struct thread *td)
> > > > +{
> > > > +	struct devreq *req;
> > > > +	device_t dev;
> > > > +	int error, old;
> > > > +
> > > > +	/* Locate the device to control. */
> > > > +	mtx_lock(&Giant);
> > > > +	req = (struct devreq *)data;
> > > 
> > > [..]
> > > 
> > > > +	switch (cmd) {
> > > 
> > > [..]
> > > 
> > > > +	case DEV_SET_DRIVER: {
> > > > +		devclass_t dc;
> > > > +		char driver[128];
> > > > +
> > > > +		error = copyinstr(req->dr_data, driver, sizeof(driver), 
NULL);
> > > 
> > > [..]
> > > 
> > > > +		if (!driver_exists(dev->parent, driver)) {
> > > > +			error = ENOENT;
> > > > +			break;
> > > > +		}
> > > 
> > > [..]
> > > 
> > > > +	}
> > > > +	mtx_unlock(&Giant);
> > > > +	return (error);
> > > > +}
> > > 
> > > I only skimmed thourgh this, will not a page fault drop + reacquire
> > > Giant
> > > lock?
> > > 
> > > iow, would not it be better to copy prior to taking the lock?
> > 
> > It won't make a difference.  All the logic is done after the copy, so it
> > is
> > still atomic.
> 
> Are devices guaranteed to be persistent?
> 
> find_device does ref them in any way, so i would assume that the dev you
> found is only stable as long as Giant lock is held.

Ah, fair enough.  Even when new-bus doesn't need Giant it would probably need 
this fix as well.


-- 
John Baldwin