cvs commit: src/sys/cam/scsi scsi_cd.c scsi_da.c src/sys/geom geom_disk.c geom_disk.h geom_subr.c

Pawel Jakub Dawidek pjd at
Wed Aug 15 17:19:11 UTC 2012

On Fri, Nov 18, 2005 at 10:19:40AM +0100, Poul-Henning Kamp wrote:
> In message <437D6AB5.7020306 at>, Nate Lawson writes:
> >> +void
> >> +disk_gone(struct disk *dp)
> >> +{
> >> +	struct g_geom *gp;
> >> +	struct g_provider *pp;
> >> +
> >> +	gp = dp->d_geom;
> >> +	if (gp != NULL)
> >> +		LIST_FOREACH(pp, &gp->provider, provider)
> >> +			g_orphan_provider(pp, ENXIO);
> >> +}
> >> +
> >
> >Does there need to be locking for this list traversal?  Couldn't 
> >disk_gone() race in parallel with a taste event if someone plugs/unplugs 
> >quickly, especially for a slow device (i.e. floppy)?
> Disk gone is called by the driver which owns struct disk, so nobody
> else has any business messing with that particular list.
> Obviously the driver needs to not stomp on itself, but Giant does
> that for CAM.

Sorry for answering to ~7 years old e-mail, but the lock is actually
necessary. Without holding the topology lock in disk_gone() we risk that
g_orphan_provider() or g_wither_provider() is more recent version will
remove provider from the list and we will use a pointer to freed
provider to find next one on the list.

Someone recently reported such panic to me and asked if
LIST_FOREACH_SAFE() would be enough to fix the problem.
Taking into account your response it seems it will be enough, but I
still think we should use the topology lock here, especially now that
CAM is not Giant-locked.

Pawel Jakub Dawidek             
FreeBSD committer               
Am I Evil? Yes, I Am!           
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url :

More information about the cvs-src mailing list