On Thu, Apr 09, 2009 at 08:48:01PM -0600, Scott Long wrote: > I've never liked the root_mount_hold() thing, but I'll get into that > below. This patch is absolutely wrong for CAM. I think it'll work > on SIMs that set PIM_SEQSCAN, but it'll almost certainly cause > root_mount_rel() to be called multiple times for drivers that don't > set it, which is the vast majority of the drivers in the tree. It > looks like it works for umass because umass only allows 1 target to > be probed on the bus. But given the multiple reports to the mailing > lists of failures caused by this revision, even if it works, it's > only be accident. > > So please back this out, it's causing numerous reports of unbootable > systems. I'm very happy to discuss the right way to get usb, umass, > and CAM to configure themselves correctly into the boot sequence. I'll > even go so far as to help get the rest of GEOM and the graid stuff > working right so we can kill this root_mount_hold() hack entirely.
Done. Thanks for the explanation, I am back from holiday next week and will catch up. Andrew > > Andrew Thompson wrote: >> Author: thompsa >> Date: Fri Apr 3 19:49:33 2009 >> New Revision: 190677 >> URL: http://svn.freebsd.org/changeset/base/190677 >> >> Log: >> Add interleaving root hold tokens from the CAM probe to disk_create and >> geom >> provider tasting. This is needed for disk attachments that happen after >> threads >> are running in the boot process. >> Tested by: rnoland >> >> Modified: >> head/sys/cam/cam_xpt.c >> head/sys/geom/geom.h >> head/sys/geom/geom_disk.c >> head/sys/geom/geom_disk.h >> head/sys/geom/geom_subr.c >> >> Modified: head/sys/cam/cam_xpt.c >> ============================================================================== >> --- head/sys/cam/cam_xpt.c Fri Apr 3 19:46:12 2009 (r190676) >> +++ head/sys/cam/cam_xpt.c Fri Apr 3 19:49:33 2009 (r190677) >> @@ -5139,6 +5139,7 @@ xpt_find_device(struct cam_et *target, l >> typedef struct { >> union ccb *request_ccb; >> struct ccb_pathinq *cpi; >> + struct root_hold_token *roothold; >> int counter; >> } xpt_scan_bus_info; >> @@ -5201,6 +5202,7 @@ xpt_scan_bus(struct cam_periph *periph, >> } >> scan_info->request_ccb = request_ccb; >> scan_info->cpi = &work_ccb->cpi; >> + scan_info->roothold = root_mount_hold("CAM", M_NOWAIT); >> /* Cache on our stack so we can work asynchronously */ >> max_target = scan_info->cpi->max_target; >> @@ -5232,6 +5234,7 @@ xpt_scan_bus(struct cam_periph *periph, >> printf("xpt_scan_bus: xpt_create_path failed" >> " with status %#x, bus scan halted\n", >> status); >> + root_mount_rel(scan_info->roothold); >> free(scan_info, M_CAMXPT); >> request_ccb->ccb_h.status = status; >> xpt_free_ccb(work_ccb); >> @@ -5240,6 +5243,7 @@ xpt_scan_bus(struct cam_periph *periph, >> } >> work_ccb = xpt_alloc_ccb_nowait(); >> if (work_ccb == NULL) { >> + root_mount_rel(scan_info->roothold); >> free(scan_info, M_CAMXPT); >> xpt_free_path(path); >> request_ccb->ccb_h.status = CAM_RESRC_UNAVAIL; >> @@ -5353,6 +5357,7 @@ xpt_scan_bus(struct cam_periph *periph, >> xpt_free_ccb(request_ccb); >> xpt_free_ccb((union ccb *)scan_info->cpi); >> request_ccb = scan_info->request_ccb; >> + root_mount_rel(scan_info->roothold); >> free(scan_info, M_CAMXPT); >> request_ccb->ccb_h.status = CAM_REQ_CMP; >> xpt_done(request_ccb); >> @@ -5372,6 +5377,7 @@ xpt_scan_bus(struct cam_periph *periph, >> xpt_free_ccb(request_ccb); >> xpt_free_ccb((union ccb *)scan_info->cpi); >> request_ccb = scan_info->request_ccb; >> + root_mount_rel(scan_info->roothold); >> free(scan_info, M_CAMXPT); >> request_ccb->ccb_h.status = status; >> xpt_done(request_ccb); >> >> Modified: head/sys/geom/geom.h >> ============================================================================== >> --- head/sys/geom/geom.h Fri Apr 3 19:46:12 2009 (r190676) >> +++ head/sys/geom/geom.h Fri Apr 3 19:49:33 2009 (r190677) >> @@ -193,6 +193,8 @@ struct g_provider { >> /* Two fields for the implementing class to use */ >> void *private; >> u_int index; >> + >> + struct root_hold_token *roothold; >> }; >> /* geom_dev.c */ >> >> Modified: head/sys/geom/geom_disk.c >> ============================================================================== >> --- head/sys/geom/geom_disk.c Fri Apr 3 19:46:12 2009 >> (r190676) >> +++ head/sys/geom/geom_disk.c Fri Apr 3 19:49:33 2009 >> (r190677) >> @@ -381,6 +381,7 @@ g_disk_create(void *arg, int flag) >> printf("GEOM: new disk %s\n", gp->name); >> dp->d_geom = gp; >> g_error_provider(pp, 0); >> + root_mount_rel(dp->d_roothold); >> } >> static void >> @@ -467,6 +468,7 @@ disk_create(struct disk *dp, int version >> dp->d_sectorsize, DEVSTAT_ALL_SUPPORTED, >> DEVSTAT_TYPE_DIRECT, DEVSTAT_PRIORITY_MAX); >> dp->d_geom = NULL; >> + dp->d_roothold = root_mount_hold(dp->d_name, M_WAITOK); >> g_disk_ident_adjust(dp->d_ident, sizeof(dp->d_ident)); >> g_post_event(g_disk_create, dp, M_WAITOK, dp, NULL); >> } >> >> Modified: head/sys/geom/geom_disk.h >> ============================================================================== >> --- head/sys/geom/geom_disk.h Fri Apr 3 19:46:12 2009 >> (r190676) >> +++ head/sys/geom/geom_disk.h Fri Apr 3 19:49:33 2009 >> (r190677) >> @@ -88,6 +88,8 @@ struct disk { >> /* Fields private to the driver */ >> void *d_drv1; >> + >> + struct root_hold_token *d_roothold; >> }; >> #define DISKFLAG_NEEDSGIANT 0x1 >> >> Modified: head/sys/geom/geom_subr.c >> ============================================================================== >> --- head/sys/geom/geom_subr.c Fri Apr 3 19:46:12 2009 >> (r190676) >> +++ head/sys/geom/geom_subr.c Fri Apr 3 19:49:33 2009 >> (r190677) >> @@ -545,6 +545,10 @@ g_new_provider_event(void *arg, int flag >> mp->taste(mp, pp, 0); >> g_topology_assert(); >> } >> + if (pp->roothold != NULL) { >> + root_mount_rel(pp->roothold); >> + pp->roothold = NULL; >> + } >> } >> @@ -581,6 +585,7 @@ g_new_providerf(struct g_geom *gp, const >> pp->stat = devstat_new_entry(pp, -1, 0, DEVSTAT_ALL_SUPPORTED, >> DEVSTAT_TYPE_DIRECT, DEVSTAT_PRIORITY_MAX); >> LIST_INSERT_HEAD(&gp->provider, pp, provider); >> + pp->roothold = root_mount_hold(pp->name, M_WAITOK); >> g_post_event(g_new_provider_event, pp, M_WAITOK, pp, gp, NULL); >> return (pp); >> } > _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
