Make the newbus subsystem Giant free by adding the new newbus sxlock.

The newbus lock is responsible for protecting newbus internIal structures,
device states and devclass flags. It is necessary to hold it when all
such datas are accessed. For the other operations, softc locking should
ensure enough protection to avoid races.

Newbus lock is automatically held when virtual operations on the device
and bus are invoked when loading the driver or when the suspend/resume
take place. For other 'spourious' operations trying to access/modify
the newbus topology, newbus lock needs to be automatically acquired and
dropped.

For the moment Giant is also acquired in some key point (modules subsystem)
in order to avoid problems before the 8.0 release as module handlers could
make assumptions about it. This Giant locking should go just after
the release happens.

Please keep in mind that the public interface can be expanded in order
to provide more support, if there are really necessities at some point
and also some bugs could arise as long as the patch needs a bit of
further testing.

Bump __FreeBSD_version in order to reflect the newbus lock introduction.

Reviewed by:    ed, hps, jhb, imp, mav, scottl
No answer by:   ariff, thompsa, yongari
Tested by:      pho,
                G. Trematerra <giovanni dot trematerra at gmail dot com>,
                Brandon Gooch <jamesbrandongooch at gmail dot com>
Sponsored by:   Yahoo! Incorporated
Approved by:	re (ksmith)

1 files changed, 19 insertions, 15 deletions

diff --git a/sys/dev/ata/ata-all.c b/sys/dev/ata/ata-all.c
index 1460a21..6696f0c 100644
--- a/sys/dev/ata/ata-all.c
+++ b/sys/dev/ata/ata-all.c
@@ -54,7 +54,6 @@ __FBSDID("$FreeBSD$");
 static  d_ioctl_t       ata_ioctl;
 static struct cdevsw ata_cdevsw = {
 	.d_version =    D_VERSION,
-	.d_flags =      D_NEEDGIANT, /* we need this as newbus isn't mpsafe */
 	.d_ioctl =      ata_ioctl,
 	.d_name =       "ata",
 };
@@ -204,7 +203,9 @@ ata_conn_event(void *context, int dummy)
 {
     device_t dev = (device_t)context;
 
+    newbus_xlock();
     ata_reinit(dev);
+    newbus_xunlock();
 }
 
 int
@@ -246,7 +247,6 @@ ata_reinit(device_t dev)
 
     /* reinit the children and delete any that fails */
     if (!device_get_children(dev, &children, &nchildren)) {
-	mtx_lock(&Giant);       /* newbus suckage it needs Giant */
 	for (i = 0; i < nchildren; i++) {
 	    /* did any children go missing ? */
 	    if (children[i] && device_is_attached(children[i]) &&
@@ -269,7 +269,6 @@ ata_reinit(device_t dev)
 	    }
 	}
 	free(children, M_TEMP);
-	mtx_unlock(&Giant);     /* newbus suckage dealt with, release Giant */
     }
 
     /* if we still have a good request put it on the queue again */
@@ -395,6 +394,7 @@ ata_ioctl(struct cdev *dev, u_long cmd, caddr_t data,
     int *value = (int *)data;
     int i, nchildren, error = ENOTTY;
 
+    newbus_xlock();
     switch (cmd) {
     case IOCATAGMAXCHANNEL:
 	/* In case we have channel 0..n this will return n+1. */
@@ -405,32 +405,40 @@ ata_ioctl(struct cdev *dev, u_long cmd, caddr_t data,
     case IOCATAREINIT:
 	if (*value >= devclass_get_maxunit(ata_devclass) ||
 	    !(device = devclass_get_device(ata_devclass, *value)) ||
-	    !device_is_attached(device))
+	    !device_is_attached(device)) {
+            newbus_xunlock();
 	    return ENXIO;
+	}
 	error = ata_reinit(device);
 	break;
 
     case IOCATAATTACH:
 	if (*value >= devclass_get_maxunit(ata_devclass) ||
 	    !(device = devclass_get_device(ata_devclass, *value)) ||
-	    !device_is_attached(device))
+	    !device_is_attached(device)) {
+            newbus_xunlock();
 	    return ENXIO;
+	}
 	error = DEVICE_ATTACH(device);
 	break;
 
     case IOCATADETACH:
 	if (*value >= devclass_get_maxunit(ata_devclass) ||
 	    !(device = devclass_get_device(ata_devclass, *value)) ||
-	    !device_is_attached(device))
+	    !device_is_attached(device)) {
+            newbus_xunlock();
 	    return ENXIO;
+	}
 	error = DEVICE_DETACH(device);
 	break;
 
     case IOCATADEVICES:
 	if (devices->channel >= devclass_get_maxunit(ata_devclass) ||
 	    !(device = devclass_get_device(ata_devclass, devices->channel)) ||
-	    !device_is_attached(device))
+	    !device_is_attached(device)) {
+            newbus_xunlock();
 	    return ENXIO;
+	}
 	bzero(devices->name[0], 32);
 	bzero(&devices->params[0], sizeof(struct ata_params));
 	bzero(devices->name[1], 32);
@@ -465,6 +473,7 @@ ata_ioctl(struct cdev *dev, u_long cmd, caddr_t data,
 	if (ata_raid_ioctl_func)
 	    error = ata_raid_ioctl_func(cmd, data);
     }
+    newbus_xunlock();
     return error;
 }
 
@@ -572,7 +581,7 @@ ata_boot_attach(void)
     struct ata_channel *ch;
     int ctlr;
 
-    mtx_lock(&Giant);       /* newbus suckage it needs Giant */
+    newbus_xlock();
 
     /* kick of probe and attach on all channels */
     for (ctlr = 0; ctlr < devclass_get_maxunit(ata_devclass); ctlr++) {
@@ -587,8 +596,7 @@ ata_boot_attach(void)
 	free(ata_delayed_attach, M_TEMP);
 	ata_delayed_attach = NULL;
     }
-
-    mtx_unlock(&Giant);     /* newbus suckage dealt with, release Giant */
+    newbus_xunlock();
 }
 
 
@@ -717,7 +725,6 @@ ata_identify(device_t dev)
     if (bootverbose)
 	device_printf(dev, "Identifying devices: %08x\n", ch->devices);
 
-    mtx_lock(&Giant);
     /* Skip existing devices. */
     if (!device_get_children(dev, &children, &nchildren)) {
 	for (i = 0; i < nchildren; i++) {
@@ -729,10 +736,8 @@ ata_identify(device_t dev)
     /* Create new devices. */
     if (bootverbose)
 	device_printf(dev, "New devices: %08x\n", n);
-    if (n == 0) {
-	mtx_unlock(&Giant);
+    if (n == 0)
 	return (0);
-    }
     for (i = 0; i < ATA_PM; ++i) {
 	if (n & (((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << i))) {
 	    int unit = -1;
@@ -775,7 +780,6 @@ ata_identify(device_t dev)
     }
     bus_generic_probe(dev);
     bus_generic_attach(dev);
-    mtx_unlock(&Giant);
     return 0;
 }