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]

On Fri, Nov 18, 2005 at 10:19:40AM +0100, Poul-Henning Kamp wrote:
> In message <437D6AB5.7020306 at root.org>, 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                       http://www.wheelsystems.com
FreeBSD committer                         http://www.FreeBSD.org
Am I Evil? Yes, I Am!                     http://tupytaj.pl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20120815/466cb06c/attachment.pgp