Wiadomość napisana przez Pawel Jakub Dawidek w dniu 7 lip 2012, o godz. 23:54: > 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.
Perhaps it wasn't your original intent, but they are spares. One of these was already reused for some other task, btw. > 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. I was not aware of that patch. What I've considered was to use attributes instead, but that would complicate notifying consumers about resizing and would require some special-casing in the attribute code. >> +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? Copy-pasto, my bad. Thanks for spotting this. >> + hh = arg; >> + pp = hh->pp; >> + size = hh->size; > > Where do you free the memory allocated for 'hh'? It should be here, and it will be added soon. >> + G_VALID_PROVIDER(pp); > > Is this your protection from a provider going away? Can you suggest a way to do it in a safe way that doesn't involve rewriting most of GEOM? >> + 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. It's not that using g_orphan_provider() would be safer here - it simply wouldn't work. The way it works is by adding providers to a queue (g_doorstep). _Providers_, and we need to orphan individual consumers. So, this would involve rewriting the orphanisation mechanism. Also, most of the classes were fixed by mav@ to handle this correctly, IIRC. >> +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()? See above. -- If you cut off my head, what would I say? Me and my head, or me and my body? _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
