linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Theodore Ts'o" <tytso@mit.edu>
To: Huang Ying <ying.huang@intel.com>
Cc: Akinobu Mita <akinobu.mita@gmail.com>,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	chris.mason@fusionio.com, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 2/9] uuid: use random32_get_bytes()
Date: Tue, 30 Oct 2012 22:38:08 -0400	[thread overview]
Message-ID: <20121031023808.GB9475@thunk.org> (raw)
In-Reply-To: <1351647337.31033.13.camel@yhuang-dev>

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.

> > 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.)

Regards,

					- Ted

  reply	other threads:[~2012-10-31  2:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1351408746-8623-1-git-send-email-akinobu.mita@gmail.com>
     [not found] ` <1351408746-8623-2-git-send-email-akinobu.mita@gmail.com>
2012-10-29 20:52   ` [PATCH 2/9] uuid: use random32_get_bytes() Theodore Ts'o
2012-10-30  1:49     ` Huang Ying
2012-10-30  4:48       ` Theodore Ts'o
2012-10-31  1:35         ` Huang Ying
2012-10-31  2:38           ` Theodore Ts'o [this message]
2012-10-31  3:06             ` Huang Ying

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121031023808.GB9475@thunk.org \
    --to=tytso@mit.edu \
    --cc=akinobu.mita@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=chris.mason@fusionio.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ying.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).