Date: Sat, 28 Feb 2009 22:07:15 +0000 (UTC) From: Alexander Motin <mav@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r189195 - head/sys/dev/ata Message-ID: <200902282207.n1SM7Fug022809@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: mav Date: Sat Feb 28 22:07:15 2009 New Revision: 189195 URL: http://svn.freebsd.org/changeset/base/189195 Log: 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. Modified: head/sys/dev/ata/ata-all.c head/sys/dev/ata/ata-all.h head/sys/dev/ata/ata-disk.c head/sys/dev/ata/ata-queue.c head/sys/dev/ata/ata-raid.c head/sys/dev/ata/atapi-cam.c head/sys/dev/ata/atapi-cd.c head/sys/dev/ata/atapi-fd.c head/sys/dev/ata/atapi-tape.c Modified: head/sys/dev/ata/ata-all.c ============================================================================== --- head/sys/dev/ata/ata-all.c Sat Feb 28 19:10:43 2009 (r189194) +++ head/sys/dev/ata/ata-all.c Sat Feb 28 22:07:15 2009 (r189195) @@ -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, 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, 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, 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); Modified: head/sys/dev/ata/ata-all.h ============================================================================== --- head/sys/dev/ata/ata-all.h Sat Feb 28 19:10:43 2009 (r189194) +++ head/sys/dev/ata/ata-all.h Sat Feb 28 22:07:15 2009 (r189195) @@ -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); Modified: head/sys/dev/ata/ata-disk.c ============================================================================== --- head/sys/dev/ata/ata-disk.c Sat Feb 28 19:10:43 2009 (r189194) +++ head/sys/dev/ata/ata-disk.c Sat Feb 28 22:07:15 2009 (r189195) @@ -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) || Modified: head/sys/dev/ata/ata-queue.c ============================================================================== --- head/sys/dev/ata/ata-queue.c Sat Feb 28 19:10:43 2009 (r189194) +++ head/sys/dev/ata/ata-queue.c Sat Feb 28 22:07:15 2009 (r189195) @@ -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); } Modified: head/sys/dev/ata/ata-raid.c ============================================================================== --- head/sys/dev/ata/ata-raid.c Sat Feb 28 19:10:43 2009 (r189194) +++ head/sys/dev/ata/ata-raid.c Sat Feb 28 22:07:15 2009 (r189195) @@ -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 * 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_soft 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_soft 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_so 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_so 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 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 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; } Modified: head/sys/dev/ata/atapi-cam.c ============================================================================== --- head/sys/dev/ata/atapi-cam.c Sat Feb 28 19:10:43 2009 (r189194) +++ head/sys/dev/ata/atapi-cam.c Sat Feb 28 22:07:15 2009 (r189195) @@ -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, dev 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, in 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); } Modified: head/sys/dev/ata/atapi-cd.c ============================================================================== --- head/sys/dev/ata/atapi-cd.c Sat Feb 28 19:10:43 2009 (r189194) +++ head/sys/dev/ata/atapi-cd.c Sat Feb 28 22:07:15 2009 (r189195) @@ -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; Modified: head/sys/dev/ata/atapi-fd.c ============================================================================== --- head/sys/dev/ata/atapi-fd.c Sat Feb 28 19:10:43 2009 (r189194) +++ head/sys/dev/ata/atapi-fd.c Sat Feb 28 22:07:15 2009 (r189195) @@ -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; Modified: head/sys/dev/ata/atapi-tape.c ============================================================================== --- head/sys/dev/ata/atapi-tape.c Sat Feb 28 19:10:43 2009 (r189194) +++ head/sys/dev/ata/atapi-tape.c Sat Feb 28 22:07:15 2009 (r189195) @@ -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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200902282207.n1SM7Fug022809>