Date: Wed, 15 Aug 2012 19:19:02 +0200 From: Pawel Jakub Dawidek <pjd@FreeBSD.org> To: Poul-Henning Kamp <phk@phk.freebsd.dk> Cc: John Polstra <jdp@FreeBSD.org>, cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, Nate Lawson <nate@root.org> Subject: Re: cvs commit: src/sys/cam/scsi scsi_cd.c scsi_da.c src/sys/geom geom_disk.c geom_disk.h geom_subr.c Message-ID: <20120815171902.GD1856@garage.freebsd.pl> In-Reply-To: <6759.1132305580@critter.freebsd.dk> References: <437D6AB5.7020306@root.org> <6759.1132305580@critter.freebsd.dk>
next in thread | previous in thread | raw e-mail | index | archive | help
--OaZoDhBhXzo6bW1J
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Nov 18, 2005 at 10:19:40AM +0100, Poul-Henning Kamp wrote:
> In message <437D6AB5.7020306@root.org>, Nate Lawson writes:
>=20
> >> +void
> >> +disk_gone(struct disk *dp)
> >> +{
> >> + struct g_geom *gp;
> >> + struct g_provider *pp;
> >> +
> >> + gp =3D dp->d_geom;
> >> + if (gp !=3D NULL)
> >> + LIST_FOREACH(pp, &gp->provider, provider)
> >> + g_orphan_provider(pp, ENXIO);
> >> +}
> >> +
> >
> >Does there need to be locking for this list traversal? Couldn't=20
> >disk_gone() race in parallel with a taste event if someone plugs/unplugs=
=20
> >quickly, especially for a slow device (i.e. floppy)?
>=20
> Disk gone is called by the driver which owns struct disk, so nobody
> else has any business messing with that particular list.
>=20
> 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.
--=20
Pawel Jakub Dawidek http://www.wheelsystems.com
FreeBSD committer http://www.FreeBSD.org
Am I Evil? Yes, I Am! http://tupytaj.pl
--OaZoDhBhXzo6bW1J
Content-Type: application/pgp-signature
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)
iEYEARECAAYFAlAr2gYACgkQForvXbEpPzTbiACgvf3TEqKh7EmzZ3nhiJ8nh/f5
NzsAn2/8vLQ8AEIC4O1WMcLjbi7Vv1U2
=DhqB
-----END PGP SIGNATURE-----
--OaZoDhBhXzo6bW1J--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120815171902.GD1856>