Revert my ata_identify()/ata_reinit() related changes: r189166, r189091

and partially r188903. Revert breaks new drives detection on reinit to the
state as it was before me, but fixes series of new bugs reported by some
people.

Unconditional queueing of ata_completed() calls can lead to deadlock if
due to timeout ata_reinit() was called at the same thread by previous
ata_completed(). Calling of ata_identify() on ata_reinit() in current
implementation opens numerous races and deadlocks.

Problems I was touching here are still exist and should be addresed, but
probably in different way.
This commit is contained in:
Alexander Motin 2009-02-28 22:07:15 +00:00
parent fef11cb704
commit ea74abd5f5
9 changed files with 69 additions and 120 deletions

View File

@ -61,6 +61,7 @@ static struct cdevsw ata_cdevsw = {
/* prototypes */
static void ata_boot_attach(void);
static device_t ata_add_child(device_t, struct ata_device *, int);
static void ata_conn_event(void *, int);
static void bswap(int8_t *, int);
static void btrim(int8_t *, int);
@ -179,7 +180,7 @@ ata_detach(device_t dev)
if (!device_get_children(dev, &children, &nchildren)) {
for (i = 0; i < nchildren; i++)
if (children[i])
ata_delete_child(dev, children[i]);
device_delete_child(dev, children[i]);
free(children, M_TEMP);
}
taskqueue_drain(taskqueue_thread, &ch->conntask);
@ -264,7 +265,7 @@ ata_reinit(device_t dev)
ata_finish(request);
request = NULL;
}
ata_delete_child(dev, children[i]);
device_delete_child(dev, children[i]);
}
}
free(children, M_TEMP);
@ -290,7 +291,7 @@ ata_reinit(device_t dev)
ATA_LOCKING(dev, ATA_LF_UNLOCK);
/* Add new children. */
ata_identify(dev);
/* ata_identify(dev); */
if (bootverbose)
device_printf(dev, "reinit done ..\n");
@ -589,44 +590,19 @@ ata_boot_attach(void)
/*
* misc support functions
*/
device_t
ata_add_child(device_t parent, int unit, int atapi)
static device_t
ata_add_child(device_t parent, struct ata_device *atadev, int unit)
{
struct ata_device *atadev;
device_t child;
int dev_unit = -1;
#ifdef ATA_STATIC_ID
if (!atapi)
dev_unit = (device_get_unit(parent) << 1) + unit;
#endif
if ((child = device_add_child(parent, NULL, dev_unit))) {
if (!(atadev = malloc(sizeof(struct ata_device),
M_ATA, M_NOWAIT | M_ZERO))) {
device_printf(parent, "out of memory\n");
device_delete_child(parent, child);
return (NULL);
}
if ((child = device_add_child(parent, NULL, unit))) {
device_set_softc(child, atadev);
device_quiet(child);
atadev->dev = child;
atadev->unit = unit;
atadev->type = atapi ? ATA_T_ATAPI : ATA_T_ATA;
atadev->max_iosize = DEV_BSIZE;
atadev->mode = ATA_PIO_MAX;
}
return (child);
}
int
ata_delete_child(device_t parent, device_t child)
{
struct ata_device *atadev = device_get_softc(child);
int res;
res = device_delete_child(parent, child);
free(atadev, M_ATA);
return (res);
return child;
}
int
@ -637,11 +613,11 @@ ata_getparam(struct ata_device *atadev, int init)
u_int8_t command = 0;
int error = ENOMEM, retries = 2;
if (atadev->type == ATA_T_ATA)
if (ch->devices & (ATA_ATA_MASTER << atadev->unit))
command = ATA_ATA_IDENTIFY;
else if (atadev->type == ATA_T_ATAPI)
if (ch->devices & (ATA_ATAPI_MASTER << atadev->unit))
command = ATA_ATAPI_IDENTIFY;
else
if (!command)
return ENXIO;
while (retries-- > 0 && error) {
@ -651,7 +627,7 @@ ata_getparam(struct ata_device *atadev, int init)
request->timeout = 1;
request->retries = 0;
request->u.ata.command = command;
request->flags = (ATA_R_READ|ATA_R_AT_HEAD|ATA_R_THREAD);
request->flags = (ATA_R_READ|ATA_R_AT_HEAD|ATA_R_DIRECT);
if (!bootverbose)
request->flags |= ATA_R_QUIET;
request->data = (void *)&atadev->param;
@ -687,18 +663,17 @@ ata_getparam(struct ata_device *atadev, int init)
btrim(atacap->serial, sizeof(atacap->serial));
bpack(atacap->serial, atacap->serial, sizeof(atacap->serial));
if (init) {
char buffer[64];
if (bootverbose) {
printf("ata%d-%s: pio=%s wdma=%s udma=%s cable=%s wire\n",
if (bootverbose)
printf("ata%d-%s: pio=%s wdma=%s udma=%s cable=%s wire\n",
device_get_unit(ch->dev),
ata_unit2str(atadev),
ata_mode2str(ata_pmode(atacap)),
ata_mode2str(ata_wmode(atacap)),
ata_mode2str(ata_umode(atacap)),
(atacap->hwres & ATA_CABLE_ID) ? "80":"40");
}
if (init) {
char buffer[64];
sprintf(buffer, "%.40s/%.8s", atacap->model, atacap->revision);
device_set_desc_copy(atadev->dev, buffer);
@ -731,6 +706,7 @@ ata_identify(device_t dev)
struct ata_channel *ch = device_get_softc(dev);
struct ata_device *atadev;
device_t *children;
device_t child;
int nchildren, i, n = ch->devices;
if (bootverbose)
@ -745,18 +721,37 @@ ata_identify(device_t dev)
}
free(children, M_TEMP);
}
/* Create new devices. */
if (bootverbose)
device_printf(dev, "New devices: %08x\n", n);
if (n == 0) {
mtx_unlock(&Giant);
return (0);
}
/* Create new devices. */
for (i = 0; i < ATA_PM; ++i) {
if (n & (((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << i)))
ata_add_child(dev, i, n & (ATA_ATAPI_MASTER << i));
}
if (n & (((ATA_ATA_MASTER | ATA_ATAPI_MASTER) << i))) {
int unit = -1;
if (!(atadev = malloc(sizeof(struct ata_device),
M_ATA, M_NOWAIT | M_ZERO))) {
device_printf(dev, "out of memory\n");
return ENOMEM;
}
atadev->unit = i;
#ifdef ATA_STATIC_ID
if (n & (ATA_ATA_MASTER << i))
unit = (device_get_unit(dev) << 1) + i;
#endif
if ((child = ata_add_child(dev, atadev, unit))) {
if (ata_getparam(atadev, 1)) {
device_delete_child(dev, child);
free(atadev, M_ATA);
}
}
else
free(atadev, M_ATA);
}
}
bus_generic_probe(dev);
bus_generic_attach(dev);
mtx_unlock(&Giant);

View File

@ -367,6 +367,7 @@ struct ata_request {
#define ATA_R_AT_HEAD 0x00000200
#define ATA_R_REQUEUE 0x00000400
#define ATA_R_THREAD 0x00000800
#define ATA_R_DIRECT 0x00001000
#define ATA_R_DEBUG 0x10000000
#define ATA_R_DANGER1 0x20000000
@ -410,10 +411,6 @@ struct ata_device {
#define ATA_MASTER 0x00
#define ATA_SLAVE 0x01
#define ATA_PM 0x0f
int type; /* device type */
#define ATA_T_ATA 0x00
#define ATA_T_ATAPI 0x01
#define ATA_T_ATAPI_CAM 0x02
struct ata_params param; /* ata param structure */
int mode; /* current transfermode */
@ -426,8 +423,6 @@ struct ata_device {
#define ATA_D_MEDIA_CHANGED 0x0002
#define ATA_D_ENC_PRESENT 0x0004
#define ATA_D_48BIT_ACTIVE 0x0008
#define ATA_D_PROBED 0x0010
#define ATA_D_VALID 0x0020
};
/* structure for holding DMA Physical Region Descriptors (PRD) entries */
@ -565,8 +560,6 @@ void ata_interrupt(void *data);
int ata_device_ioctl(device_t dev, u_long cmd, caddr_t data);
int ata_getparam(struct ata_device *atadev, int init);
int ata_identify(device_t dev);
device_t ata_add_child(device_t, int, int);
int ata_delete_child(device_t , device_t);
void ata_default_registers(device_t dev);
void ata_modify_if_48bit(struct ata_request *request);
void ata_udelay(int interval);

View File

@ -79,18 +79,6 @@ ad_probe(device_t dev)
{
struct ata_device *atadev = device_get_softc(dev);
if (atadev->type != ATA_T_ATA)
return (ENXIO);
if (!(atadev->flags & ATA_D_PROBED)) {
atadev->flags |= ATA_D_PROBED;
if (ata_getparam(atadev, 1) == 0)
atadev->flags |= ATA_D_VALID;
}
if (!(atadev->flags & ATA_D_VALID))
return (ENXIO);
if (!(atadev->param.config & ATA_PROTO_ATAPI) ||
(atadev->param.config == ATA_CFA_MAGIC1) ||
(atadev->param.config == ATA_CFA_MAGIC2) ||

View File

@ -237,8 +237,14 @@ ata_start(device_t dev)
void
ata_finish(struct ata_request *request)
{
struct ata_channel *ch = device_get_softc(request->parent);
if (dumping) {
/*
* if in ATA_STALL_QUEUE state or request has ATA_R_DIRECT flags set
* we need to call ata_complete() directly here (no taskqueue involvement)
*/
if (dumping ||
(ch->state & ATA_STALL_QUEUE) || (request->flags & ATA_R_DIRECT)) {
ATA_DEBUG_RQ(request, "finish directly");
ata_completed(request, 0);
}

View File

@ -275,7 +275,7 @@ ata_raid_flush(struct bio *bp)
request->u.ata.feature = 0;
request->timeout = 1;
request->retries = 0;
request->flags |= ATA_R_ORDERED | ATA_R_THREAD;
request->flags |= ATA_R_ORDERED | ATA_R_DIRECT;
ata_queue_request(request);
}
return 0;
@ -1570,7 +1570,7 @@ ata_raid_wipe_metadata(struct ar_softc *rdp)
if (!(meta = malloc(size, M_AR, M_NOWAIT | M_ZERO)))
return ENOMEM;
if (ata_raid_rw(rdp->disks[disk].dev, lba, meta, size,
ATA_R_WRITE | ATA_R_THREAD)) {
ATA_R_WRITE | ATA_R_DIRECT)) {
device_printf(rdp->disks[disk].dev, "wipe metadata failed\n");
error = EIO;
}
@ -2264,7 +2264,7 @@ ata_raid_hptv2_write_meta(struct ar_softc *rdp)
if (ata_raid_rw(rdp->disks[disk].dev,
HPTV2_LBA(rdp->disks[disk].dev), meta,
sizeof(struct promise_raid_conf),
ATA_R_WRITE | ATA_R_THREAD)) {
ATA_R_WRITE | ATA_R_DIRECT)) {
device_printf(rdp->disks[disk].dev, "write metadata failed\n");
error = EIO;
}
@ -2710,7 +2710,7 @@ ata_raid_intel_write_meta(struct ar_softc *rdp)
if (rdp->disks[disk].dev) {
if (ata_raid_rw(rdp->disks[disk].dev,
INTEL_LBA(rdp->disks[disk].dev),
meta, 1024, ATA_R_WRITE | ATA_R_THREAD)) {
meta, 1024, ATA_R_WRITE | ATA_R_DIRECT)) {
device_printf(rdp->disks[disk].dev, "write metadata failed\n");
error = EIO;
}
@ -3055,7 +3055,7 @@ ata_raid_jmicron_write_meta(struct ar_softc *rdp)
if (ata_raid_rw(rdp->disks[disk].dev,
JMICRON_LBA(rdp->disks[disk].dev),
meta, sizeof(struct jmicron_raid_conf),
ATA_R_WRITE | ATA_R_THREAD)) {
ATA_R_WRITE | ATA_R_DIRECT)) {
device_printf(rdp->disks[disk].dev, "write metadata failed\n");
error = EIO;
}
@ -3778,7 +3778,7 @@ ata_raid_promise_write_meta(struct ar_softc *rdp)
if (ata_raid_rw(rdp->disks[disk].dev,
PROMISE_LBA(rdp->disks[disk].dev),
meta, sizeof(struct promise_raid_conf),
ATA_R_WRITE | ATA_R_THREAD)) {
ATA_R_WRITE | ATA_R_DIRECT)) {
device_printf(rdp->disks[disk].dev, "write metadata failed\n");
error = EIO;
}
@ -4126,7 +4126,7 @@ ata_raid_sis_write_meta(struct ar_softc *rdp)
if (ata_raid_rw(rdp->disks[disk].dev,
SIS_LBA(rdp->disks[disk].dev),
meta, sizeof(struct sis_raid_conf),
ATA_R_WRITE | ATA_R_THREAD)) {
ATA_R_WRITE | ATA_R_DIRECT)) {
device_printf(rdp->disks[disk].dev, "write metadata failed\n");
error = EIO;
}
@ -4351,7 +4351,7 @@ ata_raid_via_write_meta(struct ar_softc *rdp)
if (ata_raid_rw(rdp->disks[disk].dev,
VIA_LBA(rdp->disks[disk].dev),
meta, sizeof(struct via_raid_conf),
ATA_R_WRITE | ATA_R_THREAD)) {
ATA_R_WRITE | ATA_R_DIRECT)) {
device_printf(rdp->disks[disk].dev, "write metadata failed\n");
error = EIO;
}

View File

@ -147,7 +147,7 @@ static void
atapi_cam_identify(driver_t *driver, device_t parent)
{
struct atapi_xpt_softc *scp =
malloc (sizeof (struct atapi_xpt_softc), M_ATA, M_NOWAIT|M_ZERO);
malloc (sizeof (struct atapi_xpt_softc), M_ATACAM, M_NOWAIT|M_ZERO);
device_t child;
if (scp == NULL) {
@ -159,11 +159,10 @@ atapi_cam_identify(driver_t *driver, device_t parent)
child = device_add_child(parent, "atapicam", -1);
if (child == NULL) {
printf ("atapi_cam_identify: out of memory, can't add child");
free (scp, M_ATA);
free (scp, M_ATACAM);
return;
}
scp->atapi_cam_dev.unit = -1;
scp->atapi_cam_dev.type = ATA_T_ATAPI_CAM;
scp->atapi_cam_dev.dev = child;
device_quiet(child);
device_set_softc(child, scp);
@ -175,10 +174,12 @@ atapi_cam_probe(device_t dev)
struct ata_device *atadev = device_get_softc (dev);
KASSERT(atadev != NULL, ("expect valid struct ata_device"));
if (atadev->type != ATA_T_ATAPI_CAM)
return (ENXIO);
device_set_desc(dev, "ATAPI CAM Attachment");
return (0);
if (atadev->unit < 0) {
device_set_desc(dev, "ATAPI CAM Attachment");
return (0);
} else {
return ENXIO;
}
}
static int
@ -924,8 +925,11 @@ atapi_cam_event_handler(module_t mod, int what, void *arg) {
if (devlist != NULL) {
while (devlist != NULL && devcount > 0) {
device_t child = devlist[--devcount];
struct atapi_xpt_softc *scp = device_get_softc(child);
ata_delete_child(device_get_parent(child),child);
device_delete_child(device_get_parent(child),child);
if (scp != NULL)
free(scp, M_ATACAM);
}
free(devlist, M_TEMP);
}

View File

@ -108,18 +108,6 @@ acd_probe(device_t dev)
{
struct ata_device *atadev = device_get_softc(dev);
if (atadev->type != ATA_T_ATAPI)
return (ENXIO);
if (!(atadev->flags & ATA_D_PROBED)) {
atadev->flags |= ATA_D_PROBED;
if (ata_getparam(atadev, 1) == 0)
atadev->flags |= ATA_D_VALID;
}
if (!(atadev->flags & ATA_D_VALID))
return (ENXIO);
if ((atadev->param.config & ATA_PROTO_ATAPI) &&
(atadev->param.config & ATA_ATAPI_TYPE_MASK) == ATA_ATAPI_TYPE_CDROM)
return 0;

View File

@ -66,19 +66,6 @@ static int
afd_probe(device_t dev)
{
struct ata_device *atadev = device_get_softc(dev);
if (atadev->type != ATA_T_ATAPI)
return (ENXIO);
if (!(atadev->flags & ATA_D_PROBED)) {
atadev->flags |= ATA_D_PROBED;
if (ata_getparam(atadev, 1) == 0)
atadev->flags |= ATA_D_VALID;
}
if (!(atadev->flags & ATA_D_VALID))
return (ENXIO);
if ((atadev->param.config & ATA_PROTO_ATAPI) &&
(atadev->param.config & ATA_ATAPI_TYPE_MASK) == ATA_ATAPI_TYPE_DIRECT)
return 0;

View File

@ -90,18 +90,6 @@ ast_probe(device_t dev)
{
struct ata_device *atadev = device_get_softc(dev);
if (atadev->type != ATA_T_ATAPI)
return (ENXIO);
if (!(atadev->flags & ATA_D_PROBED)) {
atadev->flags |= ATA_D_PROBED;
if (ata_getparam(atadev, 1) == 0)
atadev->flags |= ATA_D_VALID;
}
if (!(atadev->flags & ATA_D_VALID))
return (ENXIO);
if ((atadev->param.config & ATA_PROTO_ATAPI) &&
(atadev->param.config & ATA_ATAPI_TYPE_MASK) == ATA_ATAPI_TYPE_TAPE)
return 0;