All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: Will Drewry <wad@chromium.org>
Cc: Tejun Heo <tj@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"cross-distro@lists.linaro.org" <cross-distro@lists.linaro.org>,
	Jens Axboe <axboe@kernel.dk>, Kay Sievers <kay.sievers@vrfy.org>
Subject: Re: root=PARTUUID for MBR/NT disk signatures?
Date: Tue, 21 Aug 2012 12:08:10 -0600	[thread overview]
Message-ID: <5033CE8A.5090705@wwwdotorg.org> (raw)
In-Reply-To: <CABqD9hY-Z9uBcYyeCa=8f8eeXkGNjPKxbwpMPi3vs3776vF6iQ@mail.gmail.com>

On 08/20/2012 10:47 PM, Will Drewry wrote:
> On Mon, Aug 20, 2012 at 1:30 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 08/20/2012 12:22 PM, Tejun Heo wrote:
>>> Hello,
>>>
>>> On Fri, Aug 17, 2012 at 04:10:52PM -0600, Stephen Warren wrote:
>>>> I was considering extending the kernel command-line option
>>>> root=PARTUUID= to also support MBR (NT disk signatures). I was thinking
>>>> of a syntax along the lines of:
>>>>
>>>> root=PARTUUID=UUUUUUUU-PP[/PARTNROFF=%d]
>>>>
>>>> ... where UUUUUUUU is the hex representation of the NT disk signature,
>>>> and PP is the hex representation of the partition number. Like GPT,
>>>> /PARTNROFF could be used too if desired.
>>>>
>>>> Related, I was thinking of changing struct partition_meta_info's uuid
>>>> field to be a string, so that it could simply be strcmp'd against the
>>>> UUID value on the kernel command-line. That way, the type of the UUID is
>>>> irrelevant.
>>>>
>>>> Does anyone have any objection to that?
>>>
>>> Wouldn't that be able to break setups which work currently?
>>
>> I don't believe so:
>>
>> Since the newly supported UUID syntax wouldn't ever match any EFI UUID
>> (the lengths differ in all cases), I don't believe the new syntax would
>> affect behavior for any existing usage.
>>
>> Obviously, part_efi.c would be modified to initialize struct
>> partition_meta_info's uuid field to the appropriate string
>> representation of the UUID so that the str(case)cmp would still succeed
>> for existing command-lines. I ended up coding up that part of the change
>> late Friday, and the feature was certainly still working OK.
> 
> Functionally, I suspect this will work fine, but I am concerned that
> it is a bad move from an efficiency perspective (not unfixable
> though).  Right now, the user-supplied value is converted from
> string-uuid to packed-uuid.  This is then memcmp'd across any and all
> partitions - be it 2 or 200 - across all attached storage.  If we move
> to a pure string, then we end up needing to unpack every packed UUID
> at disk scan time (or search, depending on impl) rather than just the
> one user supplied value.

The EFI partition code actually does the following already:

1) Unpack the UUID from the binary on-disk representation to a temporary
string.
2) Repack the temporary string into the internal UUID buffer.

The comments imply this is in order to do endian conversions.

Switching the internal representation to a string avoids step (2) above,
plus avoids having to pack the string on the kernel command-line into a
binary UUID before the comparison. I doubt the difference between memcmp
vs. strcasecmp is worth considering. So, I think it's overall a win.

  parent reply	other threads:[~2012-08-21 18:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-17 22:10 root=PARTUUID for MBR/NT disk signatures? Stephen Warren
2012-08-20 18:22 ` Tejun Heo
2012-08-20 18:30   ` Stephen Warren
2012-08-21  4:47     ` Will Drewry
2012-08-21  6:38       ` Michael Tokarev
2012-08-21 18:08       ` Stephen Warren [this message]
2012-08-21 18:47         ` Will Drewry

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=5033CE8A.5090705@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=axboe@kernel.dk \
    --cc=cross-distro@lists.linaro.org \
    --cc=kay.sievers@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=wad@chromium.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.