From owner-svn-src-all@FreeBSD.ORG Sat Jul 7 21:56:42 2012 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id D97CA1065672; Sat, 7 Jul 2012 21:56:42 +0000 (UTC) (envelope-from pawel@dawidek.net) Received: from mail.dawidek.net (60.wheelsystems.com [83.12.187.60]) by mx1.freebsd.org (Postfix) with ESMTP id 523D18FC0A; Sat, 7 Jul 2012 21:56:42 +0000 (UTC) Received: from localhost (89-73-195-149.dynamic.chello.pl [89.73.195.149]) by mail.dawidek.net (Postfix) with ESMTPSA id 6D43D58E; Sat, 7 Jul 2012 23:56:40 +0200 (CEST) Date: Sat, 7 Jul 2012 23:54:25 +0200 From: Pawel Jakub Dawidek To: Edward Tomasz Napierala Message-ID: <20120707215424.GE1437@garage.freebsd.pl> References: <201207072013.q67KDfHN082943@svn.freebsd.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="qOrJKOH36bD5yhNe" Content-Disposition: inline In-Reply-To: <201207072013.q67KDfHN082943@svn.freebsd.org> X-OS: FreeBSD 10.0-CURRENT amd64 User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r238213 - head/sys/geom X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Jul 2012 21:56:43 -0000 --qOrJKOH36bD5yhNe Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Jul 07, 2012 at 08:13:41PM +0000, Edward Tomasz Napierala wrote: > Author: trasz > Date: Sat Jul 7 20:13:40 2012 > New Revision: 238213 > URL: http://svn.freebsd.org/changeset/base/238213 >=20 > Log: > Add a new GEOM method, resize(), which is called after provider size ch= anges. > Add a new routine, g_resize_provider(), to use to notify GEOM about pro= vider > change. > =20 > Reviewed by: mav > Sponsored by: FreeBSD Foundation [...] > - void *spare2; > + g_resize_t *resize; [...] > - void *spare1; > + g_resize_t *resize; [...] If you take the time to actually read the commit log from the change that added those spare fields, you will notice they were not added for you to consume them. You will also notice that one of those fields were left for more universal method to handle various provider's property changes (ie. provider's name, apart from its mediasize). The initial patch was even published a year ago: http://people.freebsd.org/~pjd/patches/geom_property_change.patch Even if it was somehow totally not reusable it would at least give you a hint that mediasize is not the only thing that can change and if we are making that change it should be done right. > +static void > +g_resize_provider_event(void *arg, int flag) > +{ [...] > + if (flag =3D=3D EV_CANCEL) > + return; How it can be canceled? Because I'm clearly missing something. You post this event without giving any pointers, so how g_cancel_event() can find this event can cancel it? > + hh =3D arg; > + pp =3D hh->pp; > + size =3D hh->size; Where do you free the memory allocated for 'hh'? > + G_VALID_PROVIDER(pp); Is this your protection from a provider going away? > + LIST_FOREACH_SAFE(cp, &pp->consumers, consumers, cp2) { > + gp =3D cp->geom; > + if (gp->resize =3D=3D NULL && size < pp->mediasize) > + cp->geom->orphan(cp); > + } Why is this safe to call the orphan method directly and not use g_orphan_provider()? I don't know if using g_orphan_provider() is safe to use here either, but I'm under impression that you assume no orphan method will ever drop the topology lock? We have tools to assert that, no need to introduce such weak assumptions. > +void > +g_resize_provider(struct g_provider *pp, off_t size) > +{ > + struct g_hh00 *hh; > + > + G_VALID_PROVIDER(pp); > + > + if (size =3D=3D pp->mediasize) > + return; > + > + hh =3D g_malloc(sizeof *hh, M_WAITOK | M_ZERO); > + hh->pp =3D pp; Care to explain why the provider can't disappear between now and the event thread calling g_resize_provider_event()? > + hh->size =3D size; > + g_post_event(g_resize_provider_event, hh, M_WAITOK, NULL); --=20 Pawel Jakub Dawidek http://www.wheelsystems.com FreeBSD committer http://www.FreeBSD.org Am I Evil? Yes, I Am! http://tupytaj.pl --qOrJKOH36bD5yhNe Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (FreeBSD) iEYEARECAAYFAk/4sBAACgkQForvXbEpPzTuBgCg5ONWpK+E0E3m67EjNXYif4YZ bREAoIeDpaV+3k3CFUYa+0yTeJkuz13D =WhYV -----END PGP SIGNATURE----- --qOrJKOH36bD5yhNe--