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 > > Log: > Add a new GEOM method, resize(), which is called after provider size > changes. > Add a new routine, g_resize_provider(), to use to notify GEOM about provider > change. > > 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 == 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 = arg;
> + pp = hh->pp;
> + size = 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 = cp->geom;
> + if (gp->resize == 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 == pp->mediasize)
> + return;
> +
> + hh = g_malloc(sizeof *hh, M_WAITOK | M_ZERO);
> + hh->pp = pp;
Care to explain why the provider can't disappear between now and the
event thread calling g_resize_provider_event()?
> + hh->size = size;
> + g_post_event(g_resize_provider_event, hh, M_WAITOK, NULL);
--
Pawel Jakub Dawidek http://www.wheelsystems.com
FreeBSD committer http://www.FreeBSD.org
Am I Evil? Yes, I Am! http://tupytaj.pl
pgpbbjpJU23rN.pgp
Description: PGP signature
