From owner-svn-src-all@FreeBSD.ORG Sat Jul 7 22:53:43 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 90C30106564A; Sat, 7 Jul 2012 22:53:43 +0000 (UTC) (envelope-from etnapierala@googlemail.com) Received: from mail-we0-f182.google.com (mail-we0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 90A9E8FC19; Sat, 7 Jul 2012 22:53:42 +0000 (UTC) Received: by werp13 with SMTP id p13so6873866wer.13 for ; Sat, 07 Jul 2012 15:53:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=sender:subject:mime-version:content-type:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to:x-mailer; bh=WnGBx6JrrG5XDkGySBNV7y2mTSj1mRJTUIHGlRV365M=; b=0e+FY/WZQiEDdurkQGeutpbhysKTy7ND/pWOxXH82DkDJT77jTmsw51VQ03gr9THlX ntKeypI7F9EuqsZvbvxqet65LPtIAxNEzN0QO5m01UUf6/Od65jwO/Ss52iJBg2b1CdR 5gWj2jcfP+dsnWw3mBQIO0atVwC4tUg8gnT8s9GmpdYdgZ0B8y4G6PPswHjE42hDLQnH 0hkg/ofOan9VYX43KKC/gilmMpChgFjd5rw0hmSBsqyW8t5FSd+CPGz6DhfvRckV8ZQZ evwUBPA39JXSNz/9X8X2ZNlFAqfilL51NjggHWqdk3kwdrZq/K6iAkzZPAM1xYQdwHg/ /taw== Received: by 10.180.105.163 with SMTP id gn3mr18110951wib.2.1341701621340; Sat, 07 Jul 2012 15:53:41 -0700 (PDT) Received: from [192.168.119.14] (gate19.robnet.pl. [194.105.132.219]) by mx.google.com with ESMTPS id j6sm21308287wiy.4.2012.07.07.15.53.39 (version=TLSv1/SSLv3 cipher=OTHER); Sat, 07 Jul 2012 15:53:40 -0700 (PDT) Sender: =?UTF-8?Q?Edward_Tomasz_Napiera=C5=82a?= Mime-Version: 1.0 (Apple Message framework v1278) Content-Type: text/plain; charset=iso-8859-2 From: =?iso-8859-2?Q?Edward_Tomasz_Napiera=B3a?= In-Reply-To: <20120707215424.GE1437@garage.freebsd.pl> Date: Sun, 8 Jul 2012 00:53:37 +0200 Content-Transfer-Encoding: quoted-printable Message-Id: <280C8AEE-F7E8-4AAE-87BF-E59D0249B74F@FreeBSD.org> References: <201207072013.q67KDfHN082943@svn.freebsd.org> <20120707215424.GE1437@garage.freebsd.pl> To: Pawel Jakub Dawidek X-Mailer: Apple Mail (2.1278) 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 22:53:43 -0000 Wiadomo=B6=E6 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 >>=20 >> 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. >>=20 >> Reviewed by: mav >> Sponsored by: FreeBSD Foundation > [...] >> - void *spare2; >> + g_resize_t *resize; > [...] >> - void *spare1; >> + g_resize_t *resize; > [...] >=20 > 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: >=20 > = http://people.freebsd.org/~pjd/patches/geom_property_change.patch >=20 > 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 =3D=3D EV_CANCEL) >> + return; >=20 > 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 =3D arg; >> + pp =3D hh->pp; >> + size =3D hh->size; >=20 > Where do you free the memory allocated for 'hh'? It should be here, and it will be added soon. >> + G_VALID_PROVIDER(pp); >=20 > 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 =3D cp->geom; >> + if (gp->resize =3D=3D NULL && size < pp->mediasize) >> + cp->geom->orphan(cp); >> + } >=20 > 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 =3D=3D pp->mediasize) >> + return; >> + >> + hh =3D g_malloc(sizeof *hh, M_WAITOK | M_ZERO); >> + hh->pp =3D pp; >=20 > Care to explain why the provider can't disappear between now and the > event thread calling g_resize_provider_event()? See above. --=20 If you cut off my head, what would I say? Me and my head, or me and my = body?