svn commit: r190677 - in head/sys: cam geom

[Andrew Thompson]

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);
>>  }
>