Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 8 Jul 2012 23:38:46 +0200
From:      Pawel Jakub Dawidek <pjd@FreeBSD.org>
To:        Edward Tomasz =?utf-8?Q?Napiera=C5=82a?= <trasz@FreeBSD.org>
Cc:        svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org
Subject:   Re: svn commit: r238213 - head/sys/geom
Message-ID:  <20120708213846.GF1437@garage.freebsd.pl>
In-Reply-To: <280C8AEE-F7E8-4AAE-87BF-E59D0249B74F@FreeBSD.org>
References:  <201207072013.q67KDfHN082943@svn.freebsd.org> <20120707215424.GE1437@garage.freebsd.pl> <280C8AEE-F7E8-4AAE-87BF-E59D0249B74F@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--tmoQ0UElFV5VgXgH
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Jul 08, 2012 at 12:53:37AM +0200, Edward Tomasz Napiera=C5=82a wrot=
e:
> Wiadomo=C5=9B=C4=87 napisana przez Pawel Jakub Dawidek w dniu 7 lip 2012,=
 o godz. 23:54:
> > 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.
>=20
> I was not aware of that patch. [...]

I'm afraid that's a lie. From IRC logs:

<pjd> rwatson: http://people.freebsd.org/~pjd/patches/geom_property_change.=
patch
<pjd> rwatson: Not tested.
<trasz> pjd: shouldn't there also be a flag for geom to veto resizing?
<trasz> pjd: for classes that can't handle their consumers changing size?
[the discussion was pretty long]

> [...] 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.

What attributes? The ones handled by BIO_GETATTR? They are about
something totally different.

> >> +	G_VALID_PROVIDER(pp);
> >=20
> > Is this your protection from a provider going away?
>=20
> Can you suggest a way to do it in a safe way that doesn't involve
> rewriting most of GEOM?

I can only suggest not to rewrite GEOM because you didn't take the time
to understand it.

> > 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.
>=20
> 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.

By introducing such hacks you make the code unpredictable. The way
g_orphan_provider() works is more than just calling geom's orphan
method. Also, until now, when orphan method was called it meant that
provider is going away, which is not true anymore. I'd like to believe
that you carefully analysed what you changed here is safe, but based on
your understanding of GEOM, I doubt that.

--=20
Pawel Jakub Dawidek                       http://www.wheelsystems.com
FreeBSD committer                         http://www.FreeBSD.org
Am I Evil? Yes, I Am!                     http://tupytaj.pl

--tmoQ0UElFV5VgXgH
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (FreeBSD)

iEYEARECAAYFAk/5/eUACgkQForvXbEpPzT2gACePu160ufHCMEiKK4LZhCEzXxn
yeUAoJew55E5dRqOuPa0bhnBDVIteXPz
=zkP9
-----END PGP SIGNATURE-----

--tmoQ0UElFV5VgXgH--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120708213846.GF1437>