Date:	Wed, 31 Oct 2012 11:06:22 +0800
From:	Huang Ying <ying.huang@...el.com>
To:	Theodore Ts'o <tytso@....edu>
Cc:	Akinobu Mita <akinobu.mita@...il.com>,
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	chris.mason@...ionio.com, linux-btrfs@...r.kernel.org
Subject: Re: [PATCH 2/9] uuid: use random32_get_bytes()

On Tue, 2012-10-30 at 22:38 -0400, Theodore Ts'o wrote:
> On Wed, Oct 31, 2012 at 09:35:37AM +0800, Huang Ying wrote:
> > 
> > The intention of lib/uuid.c is to unify various UUID related code, and
> > put them in same place.  In addition to UUID generation, it provide some
> > other utility and may provide/collect more in the future.  So do you
> > think it is a good idea to put generate_rand_uuid/guid into lib/uuid.c
> > and maybe change the name/prototype to make it consistent with other
> > uuid definitions?
> 
> I had trouble understanding why lib/uuid.c existed, since the only
> thing I saw was the uuid generation function.  After some more
> looking, I see you also created inline functions which wrapped
> memcmp().
> 
> The problem I have with your abstractions is that it just makes life
> more complicated for the callers.  All of the current places which use
> generate_random_uuid() merely want to fill in a unsigned char array.
> This includes btrfs, by the way, which is already using
> generate_random_uuid in some places, and I'm not sure why they are
> using uuid_le_gen(), since there doesn't seem to be any need for a
> little-endian uuid/guid here (it's just used as unique bag of bits
> which is 16 bytes long), and using uuid_le_gen() means extra memory
> has to be allocated on the stack, and then an extra memory copy is
> required.  Contrast (in fs/btrfs/root-tree.c):
> 
> 	   uuid_le uuid;
> 	   ...
> 		uuid_le_gen(&uuid);
> 		memcpy(item->uuid, uuid.b, BTRFS_UUID_SIZE);
> 
> versus, simply doing (fs/btrfs/volumes.c):
> 
> 	generate_random_uuid(fs_devices->fsid);
> 
> see which one is easier?  And after the uuid is generated, none of the
> current callers ever do any manipulation of the uuid, so there's no
> real point to play fancy typedef games; it just adds more work for no
> real gain.

If we use uuid_le when we define the data structure, life will be eaiser

struct btrfs_root_item {
	...
	uuid_le uuid;
	...
};

Then it is quite easy to use it.

uuid_le_gen(&item->uuid);

That is the intended usage model.

UUID_LE() macro definition has some user.  It makes it easier to
construct UUID/GUID defined in some specs.

> > > Using UUID vs. GUID I think makes things much clearer, since the EFI
> > > specification talks about GUID's, not UUID's, and that way we don't
> > > have to worry about people getting confused about whether they should
> > > be using the little-endian versus big-endian variant.  (And I'd love
> > > to ask to whoever wrote the EFI specification what on *Earth* were
> > > they thinking when they decided to diverge from the rest of the
> > > world....)
> > 
> > I think that is a good idea.  From Wikipedia, GUID is in native byte
> > order, while UUID is in internet byte order.
> 
> Well, technially GUID is "intel/little-endian byte order".  If someone
> tried to implement the GPT on a big-endian system, such as PowerPC,
> they would still have to use the little-endian byte order, even though
> it's not the native byte order for that architecture.  Otherwise
> devices wouldn't be portable between those systems.  (This is why I
> think the GUID was such a bad idea; everyone basically treats them as
> 16 byte octet strings, so this whole idea of "native byte order" just
> to save a few byte swaps at UUID generation time was really, IMHO, a
> very bad idea.)

Yes.  Explicit byte order is better.

Best Regards,
Huang Ying


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists