Date: Mon, 16 Mar 2015 21:20:33 -0500 From: "Matthew D. Fuller" <fullermd@over-yonder.net> To: Fabian Keil <freebsd-listen@fabiankeil.de> Cc: freebsd-geom@freebsd.org Subject: Re: RFC: Pass TRIM through GELI Message-ID: <20150317022033.GI52331@over-yonder.net> In-Reply-To: <3d5629f4.63dcb347@fabiankeil.de> References: <20150308000131.GP1742@over-yonder.net> <20150314151453.GJ24274@over-yonder.net> <501bca86.508d8e26@fabiankeil.de> <20150315145324.GA52331@over-yonder.net> <20150315182444.GB52331@over-yonder.net> <20150316010845.GA1515@garage.freebsd.pl> <20150316092126.GC52331@over-yonder.net> <20150316100030.GB1515@garage.freebsd.pl> <20150316101323.GD52331@over-yonder.net> <3d5629f4.63dcb347@fabiankeil.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Mar 16, 2015 at 03:17:58PM +0100 I heard the voice of
Fabian Keil, and lo! it spake thus:
>
> If g_eli_configure() is changed to not (try to) read and write
> metadata for onetime providers it seems to work as expected:
At a glance, that looks reasonable to me (with the usual caveat about
how I'm really only pretending that I know what I'm doing here ;). It
does mean we're twiddling some bits in memory we never initialized and
never read from after (md.md_foo) in the ONETIME case, but that
doesn't seem like it'd be harmful. It's just a few wasted
instructions in a rare, manually-triggered codepath; not IMO worth the
loss in readability of sprinkling more conditionals through it.
> A nicer looking solution would be moving all the metadata fiddling
> below a if (sc->sc_flags & G_ELI_FLAG_ONETIME) block at the end of
> the function where md.md_flags could inherit the flags from
> sc->sc_flags.
I wonder. Is it actually safe to assume they should always wind up
the same? I'd think the sc at least potentially has things set the md
doesn't. For instance, if you 'geli attach -r' something, it's
read-only THIS time, so the sc has _FLAG_RO set, but not necessary in
general, so the on-disk metadata might not? I don't actually know if
that's the case, but it sounds like a reasonable thing to do, and it
certainly sounds like a reasonable _class_ of things to do, so I'd be
hesitant about thinking that
flags = sc->flags;
mess_with(flags);
sc->flags = md->flags = flags;
would be a thing we'd want to do. I s'pose you could do it instead
with a pair of set_flags/unset_flags vars built with just the changes,
that are |='d/&='d down at the bottom, but that just sounds like it'd
make things even less readable. Maybe if we had 15 or 20 bits we
might be flipping, but we've got 2.
--
Matthew Fuller (MF4839) | fullermd@over-yonder.net
Systems/Network Administrator | http://www.over-yonder.net/~fullermd/
On the Internet, nobody can hear you scream.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150317022033.GI52331>